diff --git a/cmd/fleet/serve.go b/cmd/fleet/serve.go index ac71b63084..e7e131d963 100644 --- a/cmd/fleet/serve.go +++ b/cmd/fleet/serve.go @@ -732,6 +732,7 @@ the way that the Fleet server works. mdmPushService, cronSchedules, wstepCertManager, + eeservice.NewSCEPConfigService(logger, nil), ) if err != nil { initFatal(err, "initializing service") @@ -1163,7 +1164,7 @@ the way that the Fleet server works. // SCEP proxy (for NDES, etc.) if license.IsPremium() { - if err = service.RegisterSCEPProxy(rootMux, ds, logger); err != nil { + if err = service.RegisterSCEPProxy(rootMux, ds, logger, nil); err != nil { initFatal(err, "setup SCEP proxy") } } diff --git a/cmd/fleetctl/gitops_test.go b/cmd/fleetctl/gitops_test.go index 90e87c6ad1..58ace9dbcc 100644 --- a/cmd/fleetctl/gitops_test.go +++ b/cmd/fleetctl/gitops_test.go @@ -24,6 +24,7 @@ import ( "github.com/fleetdm/fleet/v4/server/mdm/testing_utils" "github.com/fleetdm/fleet/v4/server/mock" mdmmock "github.com/fleetdm/fleet/v4/server/mock/mdm" + scep_mock "github.com/fleetdm/fleet/v4/server/mock/scep" "github.com/fleetdm/fleet/v4/server/ptr" "github.com/fleetdm/fleet/v4/server/service" "github.com/fleetdm/fleet/v4/server/test" @@ -199,11 +200,15 @@ func TestGitOpsBasicGlobalPremium(t *testing.T) { // Cannot run t.Parallel() because it sets environment variables license := &fleet.LicenseInfo{Tier: fleet.TierPremium, Expiration: time.Now().Add(24 * time.Hour)} + scepConfig := &scep_mock.SCEPConfigService{} + scepConfig.ValidateSCEPURLFunc = func(_ context.Context, _ string) error { return nil } + scepConfig.ValidateNDESSCEPAdminURLFunc = func(_ context.Context, _ fleet.NDESSCEPProxyIntegration) error { return nil } _, ds := runServerWithMockedDS( t, &service.TestServerOpts{ - License: license, - KeyValueStore: newMemKeyValueStore(), - EnableSCEPProxy: true, + License: license, + KeyValueStore: newMemKeyValueStore(), + EnableSCEPProxy: true, + SCEPConfigService: scepConfig, }, ) diff --git a/ee/server/service/mdm_external_test.go b/ee/server/service/mdm_external_test.go index 25c58dca32..f3e26d9128 100644 --- a/ee/server/service/mdm_external_test.go +++ b/ee/server/service/mdm_external_test.go @@ -95,6 +95,7 @@ func setupMockDatastorePremiumService(t testing.TB) (*mock.Store, *eeservice.Ser nil, nil, nil, + nil, ) if err != nil { panic(err) diff --git a/ee/server/service/scep_proxy.go b/ee/server/service/scep_proxy.go index c35bc78731..b0f495f564 100644 --- a/ee/server/service/scep_proxy.go +++ b/ee/server/service/scep_proxy.go @@ -33,13 +33,23 @@ const ( NDESChallengeInvalidAfter = 57 * time.Minute ) -// NDESTimeout is the timeout for NDES requests. It is exportable for testing. -var NDESTimeout = ptr.Duration(30 * time.Second) - type scepProxyService struct { ds fleet.Datastore // info logging is implemented in the service middleware layer. debugLogger log.Logger + Timeout *time.Duration +} + +// NewSCEPProxyService creates a new scep proxy service +func NewSCEPProxyService(ds fleet.Datastore, logger log.Logger, timeout *time.Duration) scepserver.ServiceWithIdentifier { + if timeout == nil { + timeout = ptr.Duration(30 * time.Second) + } + return &scepProxyService{ + ds: ds, + debugLogger: logger, + Timeout: timeout, + } } // GetCACaps returns a list of SCEP options which are supported by the server. @@ -53,7 +63,7 @@ func (svc *scepProxyService) GetCACaps(ctx context.Context) ([]byte, error) { // Return error that implements kithttp.StatusCoder interface return nil, &scepserver.BadRequestError{Message: MessageSCEPProxyNotConfigured} } - client, err := scepclient.New(appConfig.Integrations.NDESSCEPProxy.Value.URL, svc.debugLogger, NDESTimeout) + client, err := scepclient.New(appConfig.Integrations.NDESSCEPProxy.Value.URL, svc.debugLogger, svc.Timeout) if err != nil { return nil, ctxerr.Wrap(ctx, err, "creating SCEP client") } @@ -75,7 +85,7 @@ func (svc *scepProxyService) GetCACert(ctx context.Context, message string) ([]b // Return error that implements kithttp.StatusCoder interface return nil, 0, &scepserver.BadRequestError{Message: MessageSCEPProxyNotConfigured} } - client, err := scepclient.New(appConfig.Integrations.NDESSCEPProxy.Value.URL, svc.debugLogger, NDESTimeout) + client, err := scepclient.New(appConfig.Integrations.NDESSCEPProxy.Value.URL, svc.debugLogger, svc.Timeout) if err != nil { return nil, 0, ctxerr.Wrap(ctx, err, "creating SCEP client") } @@ -141,7 +151,7 @@ func (svc *scepProxyService) PKIOperation(ctx context.Context, data []byte, iden return nil, &scepserver.BadRequestError{Message: "challenge password has expired"} } - client, err := scepclient.New(appConfig.Integrations.NDESSCEPProxy.Value.URL, svc.debugLogger, NDESTimeout) + client, err := scepclient.New(appConfig.Integrations.NDESSCEPProxy.Value.URL, svc.debugLogger, svc.Timeout) if err != nil { return nil, ctxerr.Wrap(ctx, err, "creating SCEP client") } @@ -153,28 +163,39 @@ func (svc *scepProxyService) PKIOperation(ctx context.Context, data []byte, iden return res, nil } -func (svc *scepProxyService) GetNextCACert(ctx context.Context) ([]byte, error) { +func (svc *scepProxyService) GetNextCACert(_ context.Context) ([]byte, error) { // NDES on Windows Server 2022 does not support this, as advertised via GetCACaps return nil, errors.New("GetNextCACert is not implemented for SCEP proxy") } -// NewSCEPProxyService creates a new scep proxy service -func NewSCEPProxyService(ds fleet.Datastore, logger log.Logger) scepserver.ServiceWithIdentifier { - return &scepProxyService{ - ds: ds, - debugLogger: logger, +type SCEPConfigService struct { + logger log.Logger + // Timeout is the timeout for SCEP requests. + Timeout *time.Duration +} + +func NewSCEPConfigService(logger log.Logger, timeout *time.Duration) fleet.SCEPConfigService { + if timeout == nil { + timeout = ptr.Duration(30 * time.Second) + } + return &SCEPConfigService{ + logger: logger, + Timeout: timeout, } } -func ValidateNDESSCEPAdminURL(ctx context.Context, proxy fleet.NDESSCEPProxyIntegration) error { - _, err := GetNDESSCEPChallenge(ctx, proxy) +// Compile check that SCEPConfigService implements the interface. +var _ fleet.SCEPConfigService = (*SCEPConfigService)(nil) + +func (s *SCEPConfigService) ValidateNDESSCEPAdminURL(ctx context.Context, proxy fleet.NDESSCEPProxyIntegration) error { + _, err := s.GetNDESSCEPChallenge(ctx, proxy) return err } -func GetNDESSCEPChallenge(ctx context.Context, proxy fleet.NDESSCEPProxyIntegration) (string, error) { +func (s *SCEPConfigService) GetNDESSCEPChallenge(ctx context.Context, proxy fleet.NDESSCEPProxyIntegration) (string, error) { adminURL, username, password := proxy.AdminURL, proxy.Username, proxy.Password // Get the challenge from NDES - client := fleethttp.NewClient(fleethttp.WithTimeout(*NDESTimeout)) + client := fleethttp.NewClient(fleethttp.WithTimeout(*s.Timeout)) client.Transport = ntlmssp.Negotiator{ RoundTripper: fleethttp.NewTransport(), } @@ -225,8 +246,8 @@ func GetNDESSCEPChallenge(ctx context.Context, proxy fleet.NDESSCEPProxyIntegrat return challenge, nil } -func ValidateNDESSCEPURL(ctx context.Context, proxy fleet.NDESSCEPProxyIntegration, logger log.Logger) error { - client, err := scepclient.New(proxy.URL, logger, NDESTimeout) +func (s *SCEPConfigService) ValidateSCEPURL(ctx context.Context, url string) error { + client, err := scepclient.New(url, s.logger, s.Timeout) if err != nil { return ctxerr.Wrap(ctx, err, "creating SCEP client; invalid SCEP URL; please correct and try again") } diff --git a/ee/server/service/scep_proxy_test.go b/ee/server/service/scep_proxy_test.go index f1738dcc65..fd43a15965 100644 --- a/ee/server/service/scep_proxy_test.go +++ b/ee/server/service/scep_proxy_test.go @@ -24,7 +24,7 @@ import ( ) func TestValidateNDESSCEPAdminURL(t *testing.T) { - // t.Parallel() // This test is not parallel because it changes the global NDESTimeout + t.Parallel() var returnPage func() []byte returnStatus := http.StatusOK @@ -48,21 +48,19 @@ func TestValidateNDESSCEPAdminURL(t *testing.T) { } returnStatus = http.StatusNotFound - err := ValidateNDESSCEPAdminURL(context.Background(), proxy) + logger := kitlog.NewNopLogger() + svc := NewSCEPConfigService(logger, nil) + err := svc.ValidateNDESSCEPAdminURL(context.Background(), proxy) assert.ErrorContains(t, err, "unexpected status code") returnStatus = http.StatusOK // Catch timeout issue - origNDESTimeout := NDESTimeout - NDESTimeout = ptr.Duration(1 * time.Microsecond) - t.Cleanup(func() { - NDESTimeout = origNDESTimeout - }) + svc = NewSCEPConfigService(logger, ptr.Duration(1*time.Microsecond)) wait = true - err = ValidateNDESSCEPAdminURL(context.Background(), proxy) + err = svc.ValidateNDESSCEPAdminURL(context.Background(), proxy) assert.ErrorIs(t, err, context.DeadlineExceeded) wait = false - NDESTimeout = origNDESTimeout + svc = NewSCEPConfigService(logger, nil) // We need to convert the HTML page to UTF-16 encoding, which is used by Windows servers returnPageFromFile := func(path string) []byte { @@ -81,28 +79,28 @@ func TestValidateNDESSCEPAdminURL(t *testing.T) { returnPage = func() []byte { return returnPageFromFile("./testdata/mscep_admin_cache_full.html") } - err = ValidateNDESSCEPAdminURL(context.Background(), proxy) + err = svc.ValidateNDESSCEPAdminURL(context.Background(), proxy) assert.ErrorContains(t, err, "the password cache is full") // Catch ths issue when account has insufficient permissions returnPage = func() []byte { return returnPageFromFile("./testdata/mscep_admin_insufficient_permissions.html") } - err = ValidateNDESSCEPAdminURL(context.Background(), proxy) + err = svc.ValidateNDESSCEPAdminURL(context.Background(), proxy) assert.ErrorContains(t, err, "does not have sufficient permissions") // Nothing returned returnPage = func() []byte { return []byte{} } - err = ValidateNDESSCEPAdminURL(context.Background(), proxy) + err = svc.ValidateNDESSCEPAdminURL(context.Background(), proxy) assert.ErrorContains(t, err, "could not retrieve the enrollment challenge password") // All good returnPage = func() []byte { return returnPageFromFile("./testdata/mscep_admin_password.html") } - err = ValidateNDESSCEPAdminURL(context.Background(), proxy) + err = svc.ValidateNDESSCEPAdminURL(context.Background(), proxy) assert.NoError(t, err) } @@ -113,11 +111,13 @@ func TestValidateNDESSCEPURL(t *testing.T) { proxy := fleet.NDESSCEPProxyIntegration{ URL: srv.URL + "/scep", } - err := ValidateNDESSCEPURL(context.Background(), proxy, kitlog.NewNopLogger()) + logger := kitlog.NewNopLogger() + svc := NewSCEPConfigService(logger, nil) + err := svc.ValidateSCEPURL(context.Background(), proxy.URL) assert.NoError(t, err) proxy.URL = srv.URL + "/bozo" - err = ValidateNDESSCEPURL(context.Background(), proxy, kitlog.NewNopLogger()) + err = svc.ValidateSCEPURL(context.Background(), proxy.URL) assert.ErrorContains(t, err, "could not retrieve CA certificate") } diff --git a/server/datastore/mysql/app_configs.go b/server/datastore/mysql/app_configs.go index c3b7b72df2..e2635793ed 100644 --- a/server/datastore/mysql/app_configs.go +++ b/server/datastore/mysql/app_configs.go @@ -91,23 +91,39 @@ func (ds *Datastore) saveCAAssets(ctx context.Context, tx sqlx.ExtContext, info info.Integrations.NDESSCEPProxy.Value.Password = fleet.MaskedPassword } - if info.Integrations.DigiCert.Valid { - tokensToSave := make([]fleet.CAConfigAsset, 0, len(info.Integrations.DigiCert.Value)) - for i, ca := range info.Integrations.DigiCert.Value { - if ca.APIToken != "" && ca.APIToken != fleet.MaskedPassword { - tokensToSave = append(tokensToSave, fleet.CAConfigAsset{ - Name: ca.Name, - Value: []byte(ca.APIToken), - Type: fleet.CAConfigDigiCert, - }) + if info.Integrations.DigiCert.Valid || info.Integrations.CustomSCEPProxy.Valid { + tokensToSave := make([]fleet.CAConfigAsset, 0, len(info.Integrations.DigiCert.Value)+len(info.Integrations.CustomSCEPProxy.Value)) + if info.Integrations.DigiCert.Valid { + for i, ca := range info.Integrations.DigiCert.Value { + if ca.APIToken != "" && ca.APIToken != fleet.MaskedPassword { + tokensToSave = append(tokensToSave, fleet.CAConfigAsset{ + Name: ca.Name, + Value: []byte(ca.APIToken), + Type: fleet.CAConfigDigiCert, + }) + } + info.Integrations.DigiCert.Value[i].APIToken = fleet.MaskedPassword + } + } + + if info.Integrations.CustomSCEPProxy.Valid { + for i, ca := range info.Integrations.CustomSCEPProxy.Value { + if ca.Challenge != "" && ca.Challenge != fleet.MaskedPassword { + tokensToSave = append(tokensToSave, fleet.CAConfigAsset{ + Name: ca.Name, + Value: []byte(ca.Challenge), + Type: fleet.CAConfigCustomSCEPProxy, + }) + } + info.Integrations.CustomSCEPProxy.Value[i].Challenge = fleet.MaskedPassword } - info.Integrations.DigiCert.Value[i].APIToken = fleet.MaskedPassword } err := ds.saveCAConfigAssets(ctx, tx, tokensToSave) if err != nil { - return ctxerr.Wrap(ctx, err, "saving DigiCert API tokens") + return ctxerr.Wrap(ctx, err, "saving CA assets") } } + return nil } diff --git a/server/datastore/mysql/ca_config_assets.go b/server/datastore/mysql/ca_config_assets.go index 25403a310c..e5d8abbac6 100644 --- a/server/datastore/mysql/ca_config_assets.go +++ b/server/datastore/mysql/ca_config_assets.go @@ -12,16 +12,18 @@ import ( "github.com/jmoiron/sqlx" ) -func (ds *Datastore) GetAllCAConfigAssets(ctx context.Context) (map[string]fleet.CAConfigAsset, error) { +func (ds *Datastore) GetAllCAConfigAssetsByType(ctx context.Context, assetType fleet.CAConfigAssetType) (map[string]fleet.CAConfigAsset, error) { stmt := ` SELECT - name, type, value + name, type, value FROM - ca_config_assets - ` + ca_config_assets +WHERE + type = ? + ` var res []fleet.CAConfigAsset - if err := sqlx.SelectContext(ctx, ds.reader(ctx), &res, stmt); err != nil { + if err := sqlx.SelectContext(ctx, ds.reader(ctx), &res, stmt, assetType); err != nil { return nil, ctxerr.Wrap(ctx, err, "get CA config assets") } diff --git a/server/datastore/mysql/ca_config_assets_test.go b/server/datastore/mysql/ca_config_assets_test.go index 1dbf5d4f5a..6742a90415 100644 --- a/server/datastore/mysql/ca_config_assets_test.go +++ b/server/datastore/mysql/ca_config_assets_test.go @@ -16,7 +16,7 @@ func TestCAConfigAssets(t *testing.T) { name string fn func(t *testing.T, ds *Datastore) }{ - {"GetAllCAConfigAssets", testGetAllCAConfigAssets}, + {"GetAllCAConfigAssetsByType", testGetAllCAConfigAssetsByType}, {"SaveCAConfigAssets", testSaveCAConfigAssets}, {"DeleteCAConfigAssets", testDeleteCAConfigAssets}, {"GetCAConfigAsset", testGetCAConfigAsset}, @@ -29,34 +29,104 @@ func TestCAConfigAssets(t *testing.T) { } } -func testGetAllCAConfigAssets(t *testing.T, ds *Datastore) { +func testGetAllCAConfigAssetsByType(t *testing.T, ds *Datastore) { ctx := context.Background() - // Test with empty table - should return not found error - assets, err := ds.GetAllCAConfigAssets(ctx) + // Test with empty table - should return not found error for both types + _, err := ds.GetAllCAConfigAssetsByType(ctx, fleet.CAConfigDigiCert) + assert.Error(t, err) + assert.True(t, fleet.IsNotFound(err)) + + _, err = ds.GetAllCAConfigAssetsByType(ctx, fleet.CAConfigCustomSCEPProxy) assert.Error(t, err) - assert.Empty(t, assets) assert.True(t, fleet.IsNotFound(err)) // Insert some test assets testAssets := []fleet.CAConfigAsset{ {Name: "asset1", Type: fleet.CAConfigDigiCert, Value: []byte("value1")}, {Name: "asset2", Type: fleet.CAConfigCustomSCEPProxy, Value: []byte("value2")}, + {Name: "asset3", Type: fleet.CAConfigDigiCert, Value: []byte("value3")}, } err = ds.SaveCAConfigAssets(ctx, testAssets) require.NoError(t, err) - // Test retrieving the assets - retrievedAssets, err := ds.GetAllCAConfigAssets(ctx) + // Test retrieving assets by type + digiCertAssets, err := ds.GetAllCAConfigAssetsByType(ctx, fleet.CAConfigDigiCert) require.NoError(t, err) - assert.Len(t, retrievedAssets, 2) + assert.Len(t, digiCertAssets, 2) - // Verify the assets were correctly stored and retrieved - for _, asset := range testAssets { - retrievedAsset, ok := retrievedAssets[asset.Name] - assert.True(t, ok, "Asset %s not found in retrieved assets", asset.Name) - assert.Equal(t, asset.Type, retrievedAsset.Type) - assert.Equal(t, asset.Value, retrievedAsset.Value) + // Verify only DigiCert assets were retrieved + _, ok := digiCertAssets["asset1"] + assert.True(t, ok, "Asset 'asset1' should be in DigiCert assets") + _, ok = digiCertAssets["asset3"] + assert.True(t, ok, "Asset 'asset3' should be in DigiCert assets") + _, ok = digiCertAssets["asset2"] + assert.False(t, ok, "Asset 'asset2' should not be in DigiCert assets") + + // Test retrieving assets by another type + scepProxyAssets, err := ds.GetAllCAConfigAssetsByType(ctx, fleet.CAConfigCustomSCEPProxy) + require.NoError(t, err) + assert.Len(t, scepProxyAssets, 1) + + // Verify only CustomSCEPProxy assets were retrieved + _, ok = scepProxyAssets["asset2"] + assert.True(t, ok, "Asset 'asset2' should be in CustomSCEPProxy assets") + _, ok = scepProxyAssets["asset1"] + assert.False(t, ok, "Asset 'asset1' should not be in CustomSCEPProxy assets") + _, ok = scepProxyAssets["asset3"] + assert.False(t, ok, "Asset 'asset3' should not be in CustomSCEPProxy assets") + + // Test with non-existent type - should return not found error + nonExistentAssets, err := ds.GetAllCAConfigAssetsByType(ctx, "non-existent-type") + assert.Error(t, err) + assert.Empty(t, nonExistentAssets) + assert.True(t, fleet.IsNotFound(err)) +} + +// Helper function to add test assets and verify they were added correctly +func addAndVerifyTestAssets(t *testing.T, ds *Datastore, ctx context.Context, assets []fleet.CAConfigAsset) { + err := ds.SaveCAConfigAssets(ctx, assets) + require.NoError(t, err) + + // Group assets by type + digiCertAssets := make([]fleet.CAConfigAsset, 0) + scepProxyAssets := make([]fleet.CAConfigAsset, 0) + + for _, asset := range assets { + switch asset.Type { + case fleet.CAConfigDigiCert: + digiCertAssets = append(digiCertAssets, asset) + case fleet.CAConfigCustomSCEPProxy: + scepProxyAssets = append(scepProxyAssets, asset) + default: + t.Fatalf("Unsupported asset type: %s", asset.Type) + } + } + + // Verify DigiCert assets if any + if len(digiCertAssets) > 0 { + retrievedAssets, err := ds.GetAllCAConfigAssetsByType(ctx, fleet.CAConfigDigiCert) + require.NoError(t, err) + + for _, asset := range digiCertAssets { + retrievedAsset, ok := retrievedAssets[asset.Name] + assert.True(t, ok, "DigiCert asset %s not found in retrieved assets", asset.Name) + assert.Equal(t, asset.Type, retrievedAsset.Type) + assert.Equal(t, asset.Value, retrievedAsset.Value) + } + } + + // Verify SCEP Proxy assets if any + if len(scepProxyAssets) > 0 { + retrievedAssets, err := ds.GetAllCAConfigAssetsByType(ctx, fleet.CAConfigCustomSCEPProxy) + require.NoError(t, err) + + for _, asset := range scepProxyAssets { + retrievedAsset, ok := retrievedAssets[asset.Name] + assert.True(t, ok, "SCEP Proxy asset %s not found in retrieved assets", asset.Name) + assert.Equal(t, asset.Type, retrievedAsset.Type) + assert.Equal(t, asset.Value, retrievedAsset.Value) + } } } @@ -67,18 +137,12 @@ func testSaveCAConfigAssets(t *testing.T, ds *Datastore) { err := ds.SaveCAConfigAssets(ctx, []fleet.CAConfigAsset{}) assert.NoError(t, err) - // Insert some test assets + // Insert and verify some test assets testAssets := []fleet.CAConfigAsset{ {Name: "asset1", Type: fleet.CAConfigDigiCert, Value: []byte("value1")}, {Name: "asset2", Type: fleet.CAConfigCustomSCEPProxy, Value: []byte("value2")}, } - err = ds.SaveCAConfigAssets(ctx, testAssets) - require.NoError(t, err) - - // Verify the assets were correctly stored - retrievedAssets, err := ds.GetAllCAConfigAssets(ctx) - require.NoError(t, err) - assert.Len(t, retrievedAssets, 2) + addAndVerifyTestAssets(t, ds, ctx, testAssets) // Update an existing asset and add a new one updatedAssets := []fleet.CAConfigAsset{ @@ -88,22 +152,27 @@ func testSaveCAConfigAssets(t *testing.T, ds *Datastore) { err = ds.SaveCAConfigAssets(ctx, updatedAssets) require.NoError(t, err) - // Verify the updates were correctly applied - retrievedAssets, err = ds.GetAllCAConfigAssets(ctx) + // Verify the updates were correctly applied - check DigiCert assets + digiCertAssets, err := ds.GetAllCAConfigAssetsByType(ctx, fleet.CAConfigDigiCert) require.NoError(t, err) - assert.Len(t, retrievedAssets, 3) + assert.Len(t, digiCertAssets, 1) + + // Check the new DigiCert asset + assert.Equal(t, fleet.CAConfigDigiCert, digiCertAssets["asset3"].Type) + assert.Equal(t, []byte("value3"), digiCertAssets["asset3"].Value) + + // Verify the updates were correctly applied - check SCEP Proxy assets + scepProxyAssets, err := ds.GetAllCAConfigAssetsByType(ctx, fleet.CAConfigCustomSCEPProxy) + require.NoError(t, err) + assert.Len(t, scepProxyAssets, 2) // Check the updated asset - assert.Equal(t, fleet.CAConfigCustomSCEPProxy, retrievedAssets["asset1"].Type) - assert.Equal(t, []byte("value1-updated"), retrievedAssets["asset1"].Value) - - // Check the new asset - assert.Equal(t, fleet.CAConfigDigiCert, retrievedAssets["asset3"].Type) - assert.Equal(t, []byte("value3"), retrievedAssets["asset3"].Value) + assert.Equal(t, fleet.CAConfigCustomSCEPProxy, scepProxyAssets["asset1"].Type) + assert.Equal(t, []byte("value1-updated"), scepProxyAssets["asset1"].Value) // Check the unchanged asset - assert.Equal(t, fleet.CAConfigCustomSCEPProxy, retrievedAssets["asset2"].Type) - assert.Equal(t, []byte("value2"), retrievedAssets["asset2"].Value) + assert.Equal(t, fleet.CAConfigCustomSCEPProxy, scepProxyAssets["asset2"].Type) + assert.Equal(t, []byte("value2"), scepProxyAssets["asset2"].Value) } func testDeleteCAConfigAssets(t *testing.T, ds *Datastore) { @@ -113,43 +182,47 @@ func testDeleteCAConfigAssets(t *testing.T, ds *Datastore) { err := ds.DeleteCAConfigAssets(ctx, []string{}) assert.NoError(t, err) - // Insert some test assets + // Insert and verify some test assets testAssets := []fleet.CAConfigAsset{ {Name: "asset1", Type: fleet.CAConfigDigiCert, Value: []byte("value1")}, {Name: "asset2", Type: fleet.CAConfigCustomSCEPProxy, Value: []byte("value2")}, {Name: "asset3", Type: fleet.CAConfigDigiCert, Value: []byte("value3")}, } - err = ds.SaveCAConfigAssets(ctx, testAssets) - require.NoError(t, err) - - // Verify assets were inserted - retrievedAssets, err := ds.GetAllCAConfigAssets(ctx) - require.NoError(t, err) - assert.Len(t, retrievedAssets, 3) + addAndVerifyTestAssets(t, ds, ctx, testAssets) // Delete one asset err = ds.DeleteCAConfigAssets(ctx, []string{"asset1"}) require.NoError(t, err) - // Verify the asset was deleted - retrievedAssets, err = ds.GetAllCAConfigAssets(ctx) + // Verify the asset was deleted by checking both types + // Check DigiCert assets + digiCertAssets, err := ds.GetAllCAConfigAssetsByType(ctx, fleet.CAConfigDigiCert) require.NoError(t, err) - assert.Len(t, retrievedAssets, 2) - _, ok := retrievedAssets["asset1"] - assert.False(t, ok, "Asset 'asset1' should have been deleted") - _, ok = retrievedAssets["asset2"] - assert.True(t, ok, "Asset 'asset2' should still exist") - _, ok = retrievedAssets["asset3"] - assert.True(t, ok, "Asset 'asset3' should still exist") + assert.Len(t, digiCertAssets, 1) + _, ok := digiCertAssets["asset1"] + assert.False(t, ok, "DigiCert asset 'asset1' should have been deleted") + _, ok = digiCertAssets["asset3"] + assert.True(t, ok, "DigiCert asset 'asset3' should still exist") + + // Check SCEP Proxy assets + scepProxyAssets, err := ds.GetAllCAConfigAssetsByType(ctx, fleet.CAConfigCustomSCEPProxy) + require.NoError(t, err) + assert.Len(t, scepProxyAssets, 1) + _, ok = scepProxyAssets["asset2"] + assert.True(t, ok, "SCEP Proxy asset 'asset2' should still exist") // Delete multiple assets err = ds.DeleteCAConfigAssets(ctx, []string{"asset2", "asset3"}) require.NoError(t, err) - // Verify all assets were deleted - _, err = ds.GetAllCAConfigAssets(ctx) + // Verify all assets were deleted - both types should return not found + _, err = ds.GetAllCAConfigAssetsByType(ctx, fleet.CAConfigDigiCert) assert.Error(t, err) - assert.True(t, fleet.IsNotFound(err), "Expected NotFound error, got: %v", err) + assert.True(t, fleet.IsNotFound(err), "Expected NotFound error for DigiCert assets, got: %v", err) + + _, err = ds.GetAllCAConfigAssetsByType(ctx, fleet.CAConfigCustomSCEPProxy) + assert.Error(t, err) + assert.True(t, fleet.IsNotFound(err), "Expected NotFound error for SCEP Proxy assets, got: %v", err) // Delete non-existent asset - should not error err = ds.DeleteCAConfigAssets(ctx, []string{"non-existent-asset"}) @@ -165,13 +238,12 @@ func testGetCAConfigAsset(t *testing.T, ds *Datastore) { assert.Nil(t, asset) assert.True(t, fleet.IsNotFound(err)) - // Insert some test assets + // Insert and verify some test assets testAssets := []fleet.CAConfigAsset{ {Name: "asset1", Type: fleet.CAConfigDigiCert, Value: []byte("value1")}, {Name: "asset2", Type: fleet.CAConfigCustomSCEPProxy, Value: []byte("value2")}, } - err = ds.SaveCAConfigAssets(ctx, testAssets) - require.NoError(t, err) + addAndVerifyTestAssets(t, ds, ctx, testAssets) // Test retrieving an existing asset by name and type asset, err = ds.GetCAConfigAsset(ctx, "asset1", fleet.CAConfigDigiCert) diff --git a/server/fleet/datastore.go b/server/fleet/datastore.go index 5c1c7d92a9..66acc4e5e6 100644 --- a/server/fleet/datastore.go +++ b/server/fleet/datastore.go @@ -1438,8 +1438,8 @@ type Datastore interface { // tx parameter is optional and can be used to pass an existing transaction. ReplaceMDMConfigAssets(ctx context.Context, assets []MDMConfigAsset, tx sqlx.ExtContext) error - // GetAllCAConfigAssets returns the config assets for DigiCert and custom SCEP CAs. - GetAllCAConfigAssets(ctx context.Context) (map[string]CAConfigAsset, error) + // GetAllCAConfigAssetsByType returns the config assets for DigiCert and custom SCEP CAs. + GetAllCAConfigAssetsByType(ctx context.Context, assetType CAConfigAssetType) (map[string]CAConfigAsset, error) GetCAConfigAsset(ctx context.Context, name string, assetType CAConfigAssetType) (*CAConfigAsset, error) SaveCAConfigAssets(ctx context.Context, assets []CAConfigAsset) error DeleteCAConfigAssets(ctx context.Context, names []string) error diff --git a/server/fleet/integrations.go b/server/fleet/integrations.go index 88035e3c58..8f107cbf0b 100644 --- a/server/fleet/integrations.go +++ b/server/fleet/integrations.go @@ -389,12 +389,24 @@ type NDESSCEPProxyIntegration struct { Password string `json:"password"` // not stored here -- encrypted in DB } +type SCEPConfigService interface { + ValidateNDESSCEPAdminURL(ctx context.Context, proxy NDESSCEPProxyIntegration) error + GetNDESSCEPChallenge(ctx context.Context, proxy NDESSCEPProxyIntegration) (string, error) + ValidateSCEPURL(ctx context.Context, url string) error +} + type CustomSCEPProxyIntegration struct { Name string `json:"name"` URL string `json:"url"` Challenge string `json:"challenge"` } +func (s *CustomSCEPProxyIntegration) Equals(other *CustomSCEPProxyIntegration) bool { + return s.Name == other.Name && + s.URL == other.URL && + (s.Challenge == "" || s.Challenge == MaskedPassword || s.Challenge == other.Challenge) +} + // Integrations configures the integrations with external systems. type Integrations struct { Jira []*JiraIntegration `json:"jira"` diff --git a/server/mock/datastore.go b/server/mock/datastore.go index 59cd07ca62..b1b1457b38 100644 --- a/server/mock/datastore.go +++ b/server/mock/datastore.go @@ -11,6 +11,7 @@ import ( //go:generate go run ./mockimpl/impl.go -o nanodep/storage.go "s *Storage" "github.com/fleetdm/fleet/v4/server/mdm/nanodep/storage.AllDEPStorage" //go:generate go run ./mockimpl/impl.go -o mdm/datastore_mdm_mock.go "fs *MDMAppleStore" "fleet.MDMAppleStore" //go:generate go run ./mockimpl/impl.go -o scep/depot.go "d *Depot" "depot.Depot" +//go:generate go run ./mockimpl/impl.go -o scep/config.go "s *SCEPConfigService" "fleet.SCEPConfigService" //go:generate go run ./mockimpl/impl.go -o mdm/bootstrap_package_store.go "s *MDMBootstrapPackageStore" "fleet.MDMBootstrapPackageStore" //go:generate go run ./mockimpl/impl.go -o software/software_installer_store.go "s *SoftwareInstallerStore" "fleet.SoftwareInstallerStore" diff --git a/server/mock/datastore_mock.go b/server/mock/datastore_mock.go index 9b02c8c84e..e8d2e7c93c 100644 --- a/server/mock/datastore_mock.go +++ b/server/mock/datastore_mock.go @@ -948,7 +948,7 @@ type HardDeleteMDMConfigAssetFunc func(ctx context.Context, assetName fleet.MDMA type ReplaceMDMConfigAssetsFunc func(ctx context.Context, assets []fleet.MDMConfigAsset, tx sqlx.ExtContext) error -type GetAllCAConfigAssetsFunc func(ctx context.Context) (map[string]fleet.CAConfigAsset, error) +type GetAllCAConfigAssetsByTypeFunc func(ctx context.Context, assetType fleet.CAConfigAssetType) (map[string]fleet.CAConfigAsset, error) type GetCAConfigAssetFunc func(ctx context.Context, name string, assetType fleet.CAConfigAssetType) (*fleet.CAConfigAsset, error) @@ -2666,8 +2666,8 @@ type DataStore struct { ReplaceMDMConfigAssetsFunc ReplaceMDMConfigAssetsFunc ReplaceMDMConfigAssetsFuncInvoked bool - GetAllCAConfigAssetsFunc GetAllCAConfigAssetsFunc - GetAllCAConfigAssetsFuncInvoked bool + GetAllCAConfigAssetsByTypeFunc GetAllCAConfigAssetsByTypeFunc + GetAllCAConfigAssetsByTypeFuncInvoked bool GetCAConfigAssetFunc GetCAConfigAssetFunc GetCAConfigAssetFuncInvoked bool @@ -6402,11 +6402,11 @@ func (s *DataStore) ReplaceMDMConfigAssets(ctx context.Context, assets []fleet.M return s.ReplaceMDMConfigAssetsFunc(ctx, assets, tx) } -func (s *DataStore) GetAllCAConfigAssets(ctx context.Context) (map[string]fleet.CAConfigAsset, error) { +func (s *DataStore) GetAllCAConfigAssetsByType(ctx context.Context, assetType fleet.CAConfigAssetType) (map[string]fleet.CAConfigAsset, error) { s.mu.Lock() - s.GetAllCAConfigAssetsFuncInvoked = true + s.GetAllCAConfigAssetsByTypeFuncInvoked = true s.mu.Unlock() - return s.GetAllCAConfigAssetsFunc(ctx) + return s.GetAllCAConfigAssetsByTypeFunc(ctx, assetType) } func (s *DataStore) GetCAConfigAsset(ctx context.Context, name string, assetType fleet.CAConfigAssetType) (*fleet.CAConfigAsset, error) { diff --git a/server/mock/scep/config.go b/server/mock/scep/config.go new file mode 100644 index 0000000000..b6214e3d60 --- /dev/null +++ b/server/mock/scep/config.go @@ -0,0 +1,52 @@ +// Automatically generated by mockimpl. DO NOT EDIT! + +package mock + +import ( + "context" + "sync" + + "github.com/fleetdm/fleet/v4/server/fleet" +) + +var _ fleet.SCEPConfigService = (*SCEPConfigService)(nil) + +type ValidateNDESSCEPAdminURLFunc func(ctx context.Context, proxy fleet.NDESSCEPProxyIntegration) error + +type GetNDESSCEPChallengeFunc func(ctx context.Context, proxy fleet.NDESSCEPProxyIntegration) (string, error) + +type ValidateSCEPURLFunc func(ctx context.Context, url string) error + +type SCEPConfigService struct { + ValidateNDESSCEPAdminURLFunc ValidateNDESSCEPAdminURLFunc + ValidateNDESSCEPAdminURLFuncInvoked bool + + GetNDESSCEPChallengeFunc GetNDESSCEPChallengeFunc + GetNDESSCEPChallengeFuncInvoked bool + + ValidateSCEPURLFunc ValidateSCEPURLFunc + ValidateSCEPURLFuncInvoked bool + + mu sync.Mutex +} + +func (s *SCEPConfigService) ValidateNDESSCEPAdminURL(ctx context.Context, proxy fleet.NDESSCEPProxyIntegration) error { + s.mu.Lock() + s.ValidateNDESSCEPAdminURLFuncInvoked = true + s.mu.Unlock() + return s.ValidateNDESSCEPAdminURLFunc(ctx, proxy) +} + +func (s *SCEPConfigService) GetNDESSCEPChallenge(ctx context.Context, proxy fleet.NDESSCEPProxyIntegration) (string, error) { + s.mu.Lock() + s.GetNDESSCEPChallengeFuncInvoked = true + s.mu.Unlock() + return s.GetNDESSCEPChallengeFunc(ctx, proxy) +} + +func (s *SCEPConfigService) ValidateSCEPURL(ctx context.Context, url string) error { + s.mu.Lock() + s.ValidateSCEPURLFuncInvoked = true + s.mu.Unlock() + return s.ValidateSCEPURLFunc(ctx, url) +} diff --git a/server/service/appconfig.go b/server/service/appconfig.go index 404df42b4b..0a5a254167 100644 --- a/server/service/appconfig.go +++ b/server/service/appconfig.go @@ -17,7 +17,6 @@ import ( "regexp" "strings" - eeservice "github.com/fleetdm/fleet/v4/ee/server/service" "github.com/fleetdm/fleet/v4/ee/server/service/digicert" "github.com/fleetdm/fleet/v4/pkg/optjson" "github.com/fleetdm/fleet/v4/pkg/rawjson" @@ -33,12 +32,6 @@ import ( "golang.org/x/text/unicode/norm" ) -// Functions that can be overwritten in tests -var ( - validateNDESSCEPAdminURL = eeservice.ValidateNDESSCEPAdminURL - validateNDESSCEPURL = eeservice.ValidateNDESSCEPURL -) - //////////////////////////////////////////////////////////////////////////////// // Get AppConfig //////////////////////////////////////////////////////////////////////////////// @@ -981,16 +974,16 @@ func (svc *Service) processAppConfigCAs(ctx context.Context, newAppConfig *fleet "Cannot encrypt NDES password. Missing required private key. Learn how to configure the private key here: https://fleetdm.com/learn-more-about/fleet-server-private-key") } - validateAdminURL, validateSCEPURL := false, false + validateAdminURL, validateURL := false, false newSCEPProxy := appConfig.Integrations.NDESSCEPProxy.Value if !oldAppConfig.Integrations.NDESSCEPProxy.Valid { result.ndes = caStatusAdded - validateAdminURL, validateSCEPURL = true, true + validateAdminURL, validateURL = true, true } else { oldSCEPProxy := oldAppConfig.Integrations.NDESSCEPProxy.Value if newSCEPProxy.URL != oldSCEPProxy.URL { result.ndes = caStatusEdited - validateSCEPURL = true + validateURL = true } if newSCEPProxy.AdminURL != oldSCEPProxy.AdminURL || newSCEPProxy.Username != oldSCEPProxy.Username || @@ -1001,22 +994,22 @@ func (svc *Service) processAppConfigCAs(ctx context.Context, newAppConfig *fleet } if validateAdminURL { - if err := validateNDESSCEPAdminURL(ctx, newSCEPProxy); err != nil { + if err := svc.scepConfigService.ValidateNDESSCEPAdminURL(ctx, newSCEPProxy); err != nil { invalid.Append("integrations.ndes_scep_proxy", err.Error()) } } - if validateSCEPURL { - if err := validateNDESSCEPURL(ctx, newSCEPProxy, svc.logger); err != nil { + if validateURL { + if err := svc.scepConfigService.ValidateSCEPURL(ctx, newSCEPProxy.URL); err != nil { invalid.Append("integrations.ndes_scep_proxy.url", err.Error()) } } } var ( - allCANames = make(map[string]struct{}) - additionalDigiCertValidationNeeded bool - additionalCustomSCEPValidationNeeded bool + allCANames = make(map[string]struct{}) + invalidCANames = make(map[string]struct{}) + additionalDigiCertValidationNeeded bool ) switch { @@ -1049,6 +1042,7 @@ func (svc *Service) processAppConfigCAs(ctx context.Context, newAppConfig *fleet !validateCACN(ca.CertificateCommonName, invalid) || !validateSeatID(ca.CertificateSeatID, invalid) || !validateUserPrincipalNames(ca.CertificateUserPrincipalNames, invalid) { + invalidCANames[ca.Name] = struct{}{} additionalDigiCertValidationNeeded = false continue } @@ -1069,6 +1063,54 @@ func (svc *Service) processAppConfigCAs(ctx context.Context, newAppConfig *fleet } } + // if additional DigiCert validation is needed, get the encrypted config assets from DB + if additionalDigiCertValidationNeeded { + remainingOldCAs := findWhichDigiCertCAsWereDeleted(oldAppConfig, newAppConfig, &result) + + err := svc.populateDigiCertAPITokens(ctx, remainingOldCAs) + if err != nil { + return result, ctxerr.Wrap(ctx, err, "populate API tokens") + } + for i, newCA := range appConfig.Integrations.DigiCert.Value { + var found, needToVerify bool + for _, oldCA := range remainingOldCAs { + switch { + case newCA.Equals(&oldCA): + // we clear the APIToken since we don't need to encrypt/save it + appConfig.Integrations.DigiCert.Value[i].APIToken = fleet.MaskedPassword + found = true + case newCA.Name == oldCA.Name: + // changed + found = true + needToVerify = newCA.NeedToVerify(&oldCA) + if needToVerify && (len(newCA.APIToken) == 0 || newCA.APIToken == fleet.MaskedPassword) { + invalid.Append("integrations.digicert.api_token", + fmt.Sprintf("DigiCert API token must be set when modifying name, URL, or GUID of an existing CA: %s", newCA.Name)) + break + } + result.digicert[newCA.Name] = caStatusEdited + } + } + if !found { + if len(newCA.APIToken) == 0 || newCA.APIToken == fleet.MaskedPassword { + invalid.Append("integrations.digicert.api_token", + fmt.Sprintf("DigiCert API token must be set on CA: %s", newCA.Name)) + break + } + result.digicert[newCA.Name] = caStatusAdded + needToVerify = true + } + if _, ok := result.digicert[newCA.Name]; ok && needToVerify { + err := digicert.VerifyProfileID(ctx, svc.logger, newCA) + if err != nil { + invalid.Append("integrations.digicert.profile_id", + fmt.Sprintf("Could not verify DigiCert profile ID %s for CA %s: %s", newCA.ProfileID, newCA.Name, err)) + } + } + } + } + + var additionalCustomSCEPValidationNeeded bool switch { case !newAppConfig.Integrations.CustomSCEPProxy.Set: // Nothing to set -- keep the old value @@ -1078,7 +1120,7 @@ func (svc *Service) processAppConfigCAs(ctx context.Context, newAppConfig *fleet // This issue is caused by the new DigiCert CA added above invalid.Append("integrations.digicert.name", fmt.Sprintf("Couldn’t edit certificate authority. "+ "\"%s\" name is already used by another DigiCert certificate authority. Please choose a different name and try again.", ca.Name)) - additionalDigiCertValidationNeeded = false + additionalCustomSCEPValidationNeeded = false continue } allCANames[ca.Name] = struct{}{} @@ -1102,6 +1144,7 @@ func (svc *Service) processAppConfigCAs(ctx context.Context, newAppConfig *fleet for _, ca := range newAppConfig.Integrations.CustomSCEPProxy.Value { ca.Name = fleet.Preprocess(ca.Name) if !validateCAName(ca.Name, "custom_scep_proxy", allCANames, invalid) { + invalidCANames[ca.Name] = struct{}{} additionalCustomSCEPValidationNeeded = false continue } @@ -1120,88 +1163,130 @@ func (svc *Service) processAppConfigCAs(ctx context.Context, newAppConfig *fleet } } - // if additional validation is needed, get all the encrypted config assets from DB - var assets map[string]fleet.CAConfigAsset - if additionalDigiCertValidationNeeded || additionalCustomSCEPValidationNeeded { - var err error - assets, err = svc.ds.GetAllCAConfigAssets(ctx) - if err != nil && !fleet.IsNotFound(err) { - return result, ctxerr.Wrap(ctx, err, "get all CA config assets") + if additionalCustomSCEPValidationNeeded { + remainingOldCAs := findWhichCustomSCEPCAsWereDeleted(oldAppConfig, newAppConfig, &result) + err := svc.populateCustomSCEPChallenges(ctx, remainingOldCAs) + if err != nil { + return result, ctxerr.Wrap(ctx, err, "populate challenges") } - // Note: The added/updated assets will be saved to DB in ds.SaveAppConfig method - } - - if additionalDigiCertValidationNeeded { - oldCAs := oldAppConfig.Integrations.DigiCert.Value - remainingOldCAs := make([]fleet.DigiCertIntegration, 0, len(oldAppConfig.Integrations.DigiCert.Value)) - for _, oldCA := range oldCAs { + for i, newCA := range appConfig.Integrations.CustomSCEPProxy.Value { var found bool - for _, newCA := range newAppConfig.Integrations.DigiCert.Value { - if oldCA.Name == newCA.Name { - found = true - break - } - } - if !found { - result.digicert[oldCA.Name] = caStatusDeleted - } else { - remainingOldCAs = append(remainingOldCAs, oldCA) - } - } - - for i, ca := range remainingOldCAs { - asset, ok := assets[ca.Name] - if !ok { - continue - } - remainingOldCAs[i].APIToken = string(asset.Value) - } - for i, newCA := range appConfig.Integrations.DigiCert.Value { - var found, needToVerify bool for _, oldCA := range remainingOldCAs { switch { case newCA.Equals(&oldCA): - // we clear the APIToken since we don't need to encrypt/save it - appConfig.Integrations.DigiCert.Value[i].APIToken = fleet.MaskedPassword + // we clear the Challenge since we don't need to encrypt/save it + appConfig.Integrations.CustomSCEPProxy.Value[i].Challenge = fleet.MaskedPassword found = true - needToVerify = false case newCA.Name == oldCA.Name: // changed - found = true - if newCA.URL != oldCA.URL && (len(newCA.APIToken) == 0 || newCA.APIToken == fleet.MaskedPassword) { - invalid.Append("integrations.digicert.api_token", - fmt.Sprintf("DigiCert API token must be set when modifying URL of an existing CA: %s", newCA.Name)) - break + if len(newCA.Challenge) == 0 || newCA.Challenge == fleet.MaskedPassword { + invalid.Append("integrations.custom_scep_proxy.challenge", + fmt.Sprintf("Custom SCEP challenge must be set when modifying existing CA: %s", newCA.Name)) + } else { + result.customSCEPProxy[newCA.Name] = caStatusEdited } - result.digicert[newCA.Name] = caStatusEdited - needToVerify = newCA.NeedToVerify(&oldCA) + found = true } } if !found { - if len(newCA.APIToken) == 0 || newCA.APIToken == fleet.MaskedPassword { - invalid.Append("integrations.digicert.api_token", - fmt.Sprintf("DigiCert API token must be set on CA: %s", newCA.Name)) - break + if len(newCA.Challenge) == 0 || newCA.Challenge == fleet.MaskedPassword { + invalid.Append("integrations.custom_scep_proxy.challenge", + fmt.Sprintf("Custom SCEP challenge must be set on CA: %s", newCA.Name)) + } else { + result.customSCEPProxy[newCA.Name] = caStatusAdded } - result.digicert[newCA.Name] = caStatusAdded - needToVerify = true } - if _, ok := result.digicert[newCA.Name]; ok && needToVerify { - err := digicert.VerifyProfileID(ctx, svc.logger, newCA) - if err != nil { - invalid.Append("integrations.digicert.profile_id", - fmt.Sprintf("Could not verify DigiCert profile ID %s for CA %s: %s", newCA.ProfileID, newCA.Name, err)) + // Unlike DigiCert, we always validate the connection on add/edit of custom SCEP + if status, ok := result.customSCEPProxy[newCA.Name]; ok && (status == caStatusEdited || status == caStatusAdded) { + if err := svc.scepConfigService.ValidateSCEPURL(ctx, newCA.URL); err != nil { + invalidCANames[newCA.Name] = struct{}{} + invalid.Append("integrations.custom_scep_proxy.url", err.Error()) } } } } - if additionalCustomSCEPValidationNeeded { - svc.logger.Log("msg", "TODO for #26603") + // Remove status updates from invalid CA names + for caName := range invalidCANames { + delete(result.digicert, caName) + delete(result.customSCEPProxy, caName) } + return result, nil } +func (svc *Service) populateDigiCertAPITokens(ctx context.Context, remainingOldCAs []fleet.DigiCertIntegration) error { + assets, err := svc.ds.GetAllCAConfigAssetsByType(ctx, fleet.CAConfigDigiCert) + if err != nil && !fleet.IsNotFound(err) { + return ctxerr.Wrap(ctx, err, "get DigiCert CA config assets") + } + // Note: The added/updated assets will be saved to DB in ds.SaveAppConfig method + for i, ca := range remainingOldCAs { + asset, ok := assets[ca.Name] + if !ok { + continue + } + remainingOldCAs[i].APIToken = string(asset.Value) + } + return nil +} + +func (svc *Service) populateCustomSCEPChallenges(ctx context.Context, remainingOldCAs []fleet.CustomSCEPProxyIntegration) error { + assets, err := svc.ds.GetAllCAConfigAssetsByType(ctx, fleet.CAConfigCustomSCEPProxy) + if err != nil && !fleet.IsNotFound(err) { + return ctxerr.Wrap(ctx, err, "get custom SCEP CA config assets") + } + // Note: The added/updated assets will be saved to DB in ds.SaveAppConfig method + for i, ca := range remainingOldCAs { + asset, ok := assets[ca.Name] + if !ok { + continue + } + remainingOldCAs[i].Challenge = string(asset.Value) + } + return nil +} + +func findWhichDigiCertCAsWereDeleted(oldAppConfig *fleet.AppConfig, newAppConfig *fleet.AppConfig, + result *appConfigCAStatus) []fleet.DigiCertIntegration { + remainingOldCAs := make([]fleet.DigiCertIntegration, 0, len(oldAppConfig.Integrations.DigiCert.Value)) + for _, oldCA := range oldAppConfig.Integrations.DigiCert.Value { + var found bool + for _, newCA := range newAppConfig.Integrations.DigiCert.Value { + if oldCA.Name == newCA.Name { + found = true + break + } + } + if !found { + result.digicert[oldCA.Name] = caStatusDeleted + } else { + remainingOldCAs = append(remainingOldCAs, oldCA) + } + } + return remainingOldCAs +} + +func findWhichCustomSCEPCAsWereDeleted(oldAppConfig *fleet.AppConfig, newAppConfig *fleet.AppConfig, + result *appConfigCAStatus) []fleet.CustomSCEPProxyIntegration { + remainingOldCAs := make([]fleet.CustomSCEPProxyIntegration, 0, len(oldAppConfig.Integrations.CustomSCEPProxy.Value)) + for _, oldCA := range oldAppConfig.Integrations.CustomSCEPProxy.Value { + var found bool + for _, newCA := range newAppConfig.Integrations.CustomSCEPProxy.Value { + if oldCA.Name == newCA.Name { + found = true + break + } + } + if !found { + result.customSCEPProxy[oldCA.Name] = caStatusDeleted + } else { + remainingOldCAs = append(remainingOldCAs, oldCA) + } + } + return remainingOldCAs +} + func validateCAName(name string, caType string, allCANames map[string]struct{}, invalid *fleet.InvalidArgumentError) bool { if name == "NDES" { invalid.Append("integrations."+caType+".name", "CA name cannot be NDES") @@ -1224,7 +1309,7 @@ func validateCAName(name string, caType string, allCANames map[string]struct{}, } if _, ok := allCANames[name]; ok { invalid.Append("integrations."+caType+".name", fmt.Sprintf("Couldn’t edit certificate authority. "+ - "\"%s\" name is already used by another DigiCert certificate authority. Please choose a different name and try again.", name)) + "\"%s\" name is already used by another certificate authority. Please choose a different name and try again.", name)) return false } allCANames[name] = struct{}{} diff --git a/server/service/appconfig_test.go b/server/service/appconfig_test.go index a14db0430f..d4ac69804a 100644 --- a/server/service/appconfig_test.go +++ b/server/service/appconfig_test.go @@ -18,6 +18,7 @@ import ( "time" "github.com/fleetdm/fleet/v4/pkg/optjson" + "github.com/fleetdm/fleet/v4/server" "github.com/fleetdm/fleet/v4/server/config" "github.com/fleetdm/fleet/v4/server/contexts/license" "github.com/fleetdm/fleet/v4/server/contexts/viewer" @@ -25,6 +26,7 @@ import ( nanodep_client "github.com/fleetdm/fleet/v4/server/mdm/nanodep/client" "github.com/fleetdm/fleet/v4/server/mock" nanodep_mock "github.com/fleetdm/fleet/v4/server/mock/nanodep" + scep_mock "github.com/fleetdm/fleet/v4/server/mock/scep" "github.com/fleetdm/fleet/v4/server/ptr" "github.com/fleetdm/fleet/v4/server/test" "github.com/go-kit/log" @@ -1489,6 +1491,7 @@ func TestModifyEnableAnalytics(t *testing.T) { } func TestModifyAppConfigForNDESSCEPProxy(t *testing.T) { + t.Parallel() ds := new(mock.Store) svc, ctx := newTestService(t, ds, nil, nil, &TestServerOpts{License: &fleet.LicenseInfo{Tier: fleet.TierFree}}) scepURL := "https://example.com/mscep/mscep.dll" @@ -1545,25 +1548,14 @@ func TestModifyAppConfigForNDESSCEPProxy(t *testing.T) { assert.ErrorContains(t, err, ErrMissingLicense.Error()) assert.ErrorContains(t, err, "integrations.ndes_scep_proxy") - origValidateNDESSCEPURL := validateNDESSCEPURL - origValidateNDESSCEPAdminURL := validateNDESSCEPAdminURL - t.Cleanup(func() { - validateNDESSCEPURL = origValidateNDESSCEPURL - validateNDESSCEPAdminURL = origValidateNDESSCEPAdminURL - }) - validateNDESSCEPURLCalled := false - validateNDESSCEPURL = func(_ context.Context, _ fleet.NDESSCEPProxyIntegration, _ log.Logger) error { - validateNDESSCEPURLCalled = true - return nil - } - validateNDESSCEPAdminURLCalled := false - validateNDESSCEPAdminURL = func(_ context.Context, _ fleet.NDESSCEPProxyIntegration) error { - validateNDESSCEPAdminURLCalled = true - return nil - } - fleetConfig := config.TestConfig() - svc, ctx = newTestServiceWithConfig(t, ds, fleetConfig, nil, nil, &TestServerOpts{License: &fleet.LicenseInfo{Tier: fleet.TierPremium}}) + scepConfig := &scep_mock.SCEPConfigService{} + scepConfig.ValidateSCEPURLFunc = func(_ context.Context, _ string) error { return nil } + scepConfig.ValidateNDESSCEPAdminURLFunc = func(_ context.Context, _ fleet.NDESSCEPProxyIntegration) error { return nil } + svc, ctx = newTestServiceWithConfig(t, ds, fleetConfig, nil, nil, &TestServerOpts{ + License: &fleet.LicenseInfo{Tier: fleet.TierPremium}, + SCEPConfigService: scepConfig, + }) ctx = viewer.NewContext(ctx, viewer.Viewer{User: admin}) ds.NewActivityFunc = func(ctx context.Context, user *fleet.User, activity fleet.ActivityDetails, details []byte, createdAt time.Time, @@ -1581,8 +1573,8 @@ func TestModifyAppConfigForNDESSCEPProxy(t *testing.T) { assert.Equal(t, fleet.MaskedPassword, ac.Integrations.NDESSCEPProxy.Value.Password) } checkSCEPProxy() - assert.True(t, validateNDESSCEPURLCalled) - assert.True(t, validateNDESSCEPAdminURLCalled) + assert.True(t, scepConfig.ValidateSCEPURLFuncInvoked) + assert.True(t, scepConfig.ValidateNDESSCEPAdminURLFuncInvoked) assert.True(t, ds.SaveAppConfigFuncInvoked) ds.SaveAppConfigFuncInvoked = false assert.True(t, ds.NewActivityFuncInvoked) @@ -1590,25 +1582,25 @@ func TestModifyAppConfigForNDESSCEPProxy(t *testing.T) { // Validation not done if there is no change appConfig = ac - validateNDESSCEPURLCalled = false - validateNDESSCEPAdminURLCalled = false + scepConfig.ValidateSCEPURLFuncInvoked = false + scepConfig.ValidateNDESSCEPAdminURLFuncInvoked = false jsonPayload = fmt.Sprintf(jsonPayloadBase, " "+scepURL, adminURL+" ", " "+username+" ", fleet.MaskedPassword) ac, err = svc.ModifyAppConfig(ctx, []byte(jsonPayload), fleet.ApplySpecOptions{}) require.NoError(t, err, jsonPayload) checkSCEPProxy() - assert.False(t, validateNDESSCEPURLCalled) - assert.False(t, validateNDESSCEPAdminURLCalled) + assert.False(t, scepConfig.ValidateSCEPURLFuncInvoked) + assert.False(t, scepConfig.ValidateNDESSCEPAdminURLFuncInvoked) assert.False(t, ds.NewActivityFuncInvoked) ds.NewActivityFuncInvoked = false // Validation not done if there is no change, part 2 - validateNDESSCEPURLCalled = false - validateNDESSCEPAdminURLCalled = false + scepConfig.ValidateSCEPURLFuncInvoked = false + scepConfig.ValidateNDESSCEPAdminURLFuncInvoked = false ac, err = svc.ModifyAppConfig(ctx, []byte(`{"integrations":{}}`), fleet.ApplySpecOptions{}) require.NoError(t, err) checkSCEPProxy() - assert.False(t, validateNDESSCEPURLCalled) - assert.False(t, validateNDESSCEPAdminURLCalled) + assert.False(t, scepConfig.ValidateSCEPURLFuncInvoked) + assert.False(t, scepConfig.ValidateNDESSCEPAdminURLFuncInvoked) assert.False(t, ds.NewActivityFuncInvoked) ds.NewActivityFuncInvoked = false @@ -1624,11 +1616,11 @@ func TestModifyAppConfigForNDESSCEPProxy(t *testing.T) { ac, err = svc.ModifyAppConfig(ctx, []byte(jsonPayload), fleet.ApplySpecOptions{}) require.NoError(t, err) checkSCEPProxy() - assert.True(t, validateNDESSCEPURLCalled) - assert.False(t, validateNDESSCEPAdminURLCalled) + assert.True(t, scepConfig.ValidateSCEPURLFuncInvoked) + assert.False(t, scepConfig.ValidateNDESSCEPAdminURLFuncInvoked) appConfig = ac - validateNDESSCEPURLCalled = false - validateNDESSCEPAdminURLCalled = false + scepConfig.ValidateSCEPURLFuncInvoked = false + scepConfig.ValidateNDESSCEPAdminURLFuncInvoked = false assert.True(t, ds.NewActivityFuncInvoked) ds.NewActivityFuncInvoked = false @@ -1638,46 +1630,36 @@ func TestModifyAppConfigForNDESSCEPProxy(t *testing.T) { ac, err = svc.ModifyAppConfig(ctx, []byte(jsonPayload), fleet.ApplySpecOptions{}) require.NoError(t, err) checkSCEPProxy() - assert.False(t, validateNDESSCEPURLCalled) - assert.True(t, validateNDESSCEPAdminURLCalled) + assert.False(t, scepConfig.ValidateSCEPURLFuncInvoked) + assert.True(t, scepConfig.ValidateNDESSCEPAdminURLFuncInvoked) assert.True(t, ds.NewActivityFuncInvoked) ds.NewActivityFuncInvoked = false // Validation fails - validateNDESSCEPURLCalled = false - validateNDESSCEPAdminURLCalled = false - validateNDESSCEPURL = func(_ context.Context, _ fleet.NDESSCEPProxyIntegration, _ log.Logger) error { - validateNDESSCEPURLCalled = true + scepConfig.ValidateSCEPURLFuncInvoked = false + scepConfig.ValidateNDESSCEPAdminURLFuncInvoked = false + scepConfig.ValidateSCEPURLFunc = func(_ context.Context, _ string) error { return errors.New("**invalid** 1") } - validateNDESSCEPAdminURL = func(_ context.Context, _ fleet.NDESSCEPProxyIntegration) error { - validateNDESSCEPAdminURLCalled = true + scepConfig.ValidateNDESSCEPAdminURLFunc = func(_ context.Context, _ fleet.NDESSCEPProxyIntegration) error { return errors.New("**invalid** 2") } scepURL = "https://new2.com/mscep/mscep.dll" jsonPayload = fmt.Sprintf(jsonPayloadBase, scepURL, adminURL, username, password) ac, err = svc.ModifyAppConfig(ctx, []byte(jsonPayload), fleet.ApplySpecOptions{}) assert.ErrorContains(t, err, "**invalid**") - assert.True(t, validateNDESSCEPURLCalled) - assert.True(t, validateNDESSCEPAdminURLCalled) + assert.True(t, scepConfig.ValidateSCEPURLFuncInvoked) + assert.True(t, scepConfig.ValidateNDESSCEPAdminURLFuncInvoked) assert.False(t, ds.NewActivityFuncInvoked) ds.NewActivityFuncInvoked = false // Reset validation - validateNDESSCEPURLCalled = false - validateNDESSCEPURL = func(_ context.Context, _ fleet.NDESSCEPProxyIntegration, _ log.Logger) error { - validateNDESSCEPURLCalled = true - return nil - } - validateNDESSCEPAdminURLCalled = false - validateNDESSCEPAdminURL = func(_ context.Context, _ fleet.NDESSCEPProxyIntegration) error { - validateNDESSCEPAdminURLCalled = true - return nil - } + scepConfig.ValidateSCEPURLFuncInvoked = false + scepConfig.ValidateNDESSCEPAdminURLFuncInvoked = false + scepConfig.ValidateSCEPURLFunc = func(_ context.Context, _ string) error { return nil } + scepConfig.ValidateNDESSCEPAdminURLFunc = func(_ context.Context, _ fleet.NDESSCEPProxyIntegration) error { return nil } // Config cleared with explicit null - validateNDESSCEPURLCalled = false - validateNDESSCEPAdminURLCalled = false payload := ` { "integrations": { @@ -1692,8 +1674,8 @@ func TestModifyAppConfigForNDESSCEPProxy(t *testing.T) { assert.False(t, ac.Integrations.NDESSCEPProxy.Valid) // Also check what was saved. assert.False(t, appConfig.Integrations.NDESSCEPProxy.Valid) - assert.False(t, validateNDESSCEPURLCalled) - assert.False(t, validateNDESSCEPAdminURLCalled) + assert.False(t, scepConfig.ValidateSCEPURLFuncInvoked) + assert.False(t, scepConfig.ValidateNDESSCEPAdminURLFuncInvoked) assert.False(t, ds.HardDeleteMDMConfigAssetFuncInvoked, "DB write should not happen in dry run") assert.False(t, ds.NewActivityFuncInvoked) ds.NewActivityFuncInvoked = false @@ -1714,8 +1696,8 @@ func TestModifyAppConfigForNDESSCEPProxy(t *testing.T) { assert.False(t, ac.Integrations.NDESSCEPProxy.Valid) // Also check what was saved. assert.False(t, appConfig.Integrations.NDESSCEPProxy.Valid) - assert.False(t, validateNDESSCEPURLCalled) - assert.False(t, validateNDESSCEPAdminURLCalled) + assert.False(t, scepConfig.ValidateSCEPURLFuncInvoked) + assert.False(t, scepConfig.ValidateNDESSCEPAdminURLFuncInvoked) assert.True(t, ds.HardDeleteMDMConfigAssetFuncInvoked) ds.HardDeleteMDMConfigAssetFuncInvoked = false assert.True(t, ds.NewActivityFuncInvoked) @@ -1727,8 +1709,8 @@ func TestModifyAppConfigForNDESSCEPProxy(t *testing.T) { 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, scepConfig.ValidateSCEPURLFuncInvoked) + assert.False(t, scepConfig.ValidateNDESSCEPAdminURLFuncInvoked) assert.False(t, ds.HardDeleteMDMConfigAssetFuncInvoked) ds.HardDeleteMDMConfigAssetFuncInvoked = false assert.False(t, ds.NewActivityFuncInvoked) @@ -1770,17 +1752,8 @@ func TestAppConfigCAs(t *testing.T) { })) defer mockDigiCertServer.Close() - type myTest struct { - ctx context.Context - svc *Service - appConfig *fleet.AppConfig - newAppConfig *fleet.AppConfig - oldAppConfig *fleet.AppConfig - invalid *fleet.InvalidArgumentError - } - - setUp := func() myTest { - mt := myTest{ + setUpDigiCert := func() configCASuite { + mt := configCASuite{ ctx: license.NewContext(context.Background(), &fleet.LicenseInfo{Tier: fleet.TierPremium}), invalid: &fleet.InvalidArgumentError{}, newAppConfig: getAppConfigWithDigiCertIntegration(mockDigiCertServer.URL, "WIFI"), @@ -1789,22 +1762,28 @@ func TestAppConfigCAs(t *testing.T) { svc: &Service{logger: log.NewLogfmtLogger(os.Stdout)}, } mt.svc.config.Server.PrivateKey = "exists" - mockDS := &mock.Store{} - mt.svc.ds = mockDS - mockDS.GetAllCAConfigAssetsFunc = func(ctx context.Context) (map[string]fleet.CAConfigAsset, error) { - return map[string]fleet.CAConfigAsset{ - "WIFI": { - Name: "WIFI", - Value: []byte("api_token"), - Type: fleet.CAConfigDigiCert, - }, - }, nil + addMockDatastoreForCA(t, mt) + return mt + } + setUpCustomSCEP := func() configCASuite { + mt := configCASuite{ + ctx: license.NewContext(context.Background(), &fleet.LicenseInfo{Tier: fleet.TierPremium}), + invalid: &fleet.InvalidArgumentError{}, + newAppConfig: getAppConfigWithSCEPIntegration("https://example.com", "SCEP_WIFI"), + oldAppConfig: &fleet.AppConfig{}, + appConfig: &fleet.AppConfig{}, + svc: &Service{logger: log.NewLogfmtLogger(os.Stdout)}, } + mt.svc.config.Server.PrivateKey = "exists" + scepConfig := &scep_mock.SCEPConfigService{} + scepConfig.ValidateSCEPURLFunc = func(_ context.Context, _ string) error { return nil } + mt.svc.scepConfigService = scepConfig + addMockDatastoreForCA(t, mt) return mt } t.Run("free license", func(t *testing.T) { - mt := setUp() + mt := setUpDigiCert() mt.ctx = license.NewContext(context.Background(), &fleet.LicenseInfo{Tier: fleet.TierFree}) mt.newAppConfig = &fleet.AppConfig{} status, err := mt.svc.processAppConfigCAs(mt.ctx, mt.newAppConfig, mt.oldAppConfig, mt.appConfig, mt.invalid) @@ -1832,7 +1811,7 @@ func TestAppConfigCAs(t *testing.T) { }) t.Run("digicert keep old value", func(t *testing.T) { - mt := setUp() + mt := setUpDigiCert() mt.ctx = license.NewContext(context.Background(), &fleet.LicenseInfo{Tier: fleet.TierPremium}) mt.oldAppConfig = mt.newAppConfig mt.appConfig = mt.oldAppConfig.Copy() @@ -1846,17 +1825,36 @@ func TestAppConfigCAs(t *testing.T) { assert.Len(t, mt.appConfig.Integrations.DigiCert.Value, 1) }) + t.Run("custom_scep keep old value", func(t *testing.T) { + mt := setUpCustomSCEP() + mt.ctx = license.NewContext(context.Background(), &fleet.LicenseInfo{Tier: fleet.TierPremium}) + mt.oldAppConfig = mt.newAppConfig + mt.appConfig = mt.oldAppConfig.Copy() + mt.newAppConfig = &fleet.AppConfig{} + status, err := mt.svc.processAppConfigCAs(mt.ctx, mt.newAppConfig, mt.oldAppConfig, mt.appConfig, mt.invalid) + require.NoError(t, err) + assert.Empty(t, mt.invalid.Errors) + assert.Empty(t, status.ndes) + assert.Empty(t, status.digicert) + assert.Empty(t, status.customSCEPProxy) + assert.Len(t, mt.appConfig.Integrations.CustomSCEPProxy.Value, 1) + }) + t.Run("missing server private key", func(t *testing.T) { - mt := setUp() + mt := setUpDigiCert() mt.svc.config.Server.PrivateKey = "" status, err := mt.svc.processAppConfigCAs(mt.ctx, mt.newAppConfig, mt.oldAppConfig, mt.appConfig, mt.invalid) require.NoError(t, err) checkExpectedCAValidationError(t, mt.invalid, status, "integrations.digicert", "private key") - // TODO: Test custom SCEP + mt = setUpCustomSCEP() + mt.svc.config.Server.PrivateKey = "" + status, err = mt.svc.processAppConfigCAs(mt.ctx, mt.newAppConfig, mt.oldAppConfig, mt.appConfig, mt.invalid) + require.NoError(t, err) + checkExpectedCAValidationError(t, mt.invalid, status, "integrations.custom_scep_proxy", "private key") }) - t.Run("invalid digicert integration name", func(t *testing.T) { + t.Run("invalid integration name", func(t *testing.T) { testCases := []struct { testName string name string @@ -1865,45 +1863,56 @@ func TestAppConfigCAs(t *testing.T) { { testName: "empty", name: "", - errorContains: []string{"integrations.digicert.name", "CA name cannot be empty"}, + errorContains: []string{"CA name cannot be empty"}, }, { testName: "NDES", name: "NDES", - errorContains: []string{"integrations.digicert.name", "CA name cannot be NDES"}, + errorContains: []string{"CA name cannot be NDES"}, }, { testName: "too long", name: strings.Repeat("a", 256), - errorContains: []string{"integrations.digicert.name", "CA name cannot be longer than"}, + errorContains: []string{"CA name cannot be longer than"}, }, { testName: "invalid characters", name: "a/b", - errorContains: []string{"integrations.digicert.name", "Only letters, numbers and underscores allowed"}, + errorContains: []string{"Only letters, numbers and underscores allowed"}, }, } for _, tc := range testCases { t.Run(tc.testName, func(t *testing.T) { - mt := setUp() + baseErrorContains := tc.errorContains + mt := setUpDigiCert() mt.newAppConfig = getAppConfigWithDigiCertIntegration(mockDigiCertServer.URL, tc.name) status, err := mt.svc.processAppConfigCAs(mt.ctx, mt.newAppConfig, mt.oldAppConfig, mt.appConfig, mt.invalid) require.NoError(t, err) - checkExpectedCAValidationError(t, mt.invalid, status, tc.errorContains...) + errorContains := baseErrorContains + errorContains = append(errorContains, "integrations.digicert.name") + checkExpectedCAValidationError(t, mt.invalid, status, errorContains...) + + mt = setUpCustomSCEP() + mt.newAppConfig = getAppConfigWithSCEPIntegration("https://example.com", tc.name) + status, err = mt.svc.processAppConfigCAs(mt.ctx, mt.newAppConfig, mt.oldAppConfig, mt.appConfig, mt.invalid) + require.NoError(t, err) + errorContains = baseErrorContains + errorContains = append(errorContains, "integrations.custom_scep_proxy.name") + checkExpectedCAValidationError(t, mt.invalid, status, errorContains...) }) } }) t.Run("invalid digicert URL", func(t *testing.T) { - mt := setUp() + mt := setUpDigiCert() mt.newAppConfig.Integrations.DigiCert.Value[0].URL = "" status, err := mt.svc.processAppConfigCAs(mt.ctx, mt.newAppConfig, mt.oldAppConfig, mt.appConfig, mt.invalid) require.NoError(t, err) checkExpectedCAValidationError(t, mt.invalid, status, "integrations.digicert.url", "empty url") - mt = setUp() + mt = setUpDigiCert() mt.newAppConfig.Integrations.DigiCert.Value[0].URL = "nonhttp://bad.com" status, err = mt.svc.processAppConfigCAs(mt.ctx, mt.newAppConfig, mt.oldAppConfig, mt.appConfig, mt.invalid) require.NoError(t, err) @@ -1911,18 +1920,55 @@ func TestAppConfigCAs(t *testing.T) { "URL must be https or http") }) + t.Run("invalid custom_scep URL", func(t *testing.T) { + mt := setUpCustomSCEP() + mt.newAppConfig.Integrations.CustomSCEPProxy.Value[0].URL = "" + status, err := mt.svc.processAppConfigCAs(mt.ctx, mt.newAppConfig, mt.oldAppConfig, mt.appConfig, mt.invalid) + require.NoError(t, err) + checkExpectedCAValidationError(t, mt.invalid, status, "integrations.custom_scep_proxy.url", + "empty url") + + mt = setUpCustomSCEP() + mt.newAppConfig.Integrations.CustomSCEPProxy.Value[0].URL = "nonhttp://bad.com" + status, err = mt.svc.processAppConfigCAs(mt.ctx, mt.newAppConfig, mt.oldAppConfig, mt.appConfig, mt.invalid) + require.NoError(t, err) + checkExpectedCAValidationError(t, mt.invalid, status, "integrations.custom_scep_proxy.url", + "URL must be https or http") + }) + t.Run("duplicate digicert integration name", func(t *testing.T) { - mt := setUp() + mt := setUpDigiCert() mt.newAppConfig.Integrations.DigiCert.Value = append(mt.newAppConfig.Integrations.DigiCert.Value, mt.newAppConfig.Integrations.DigiCert.Value[0]) status, err := mt.svc.processAppConfigCAs(mt.ctx, mt.newAppConfig, mt.oldAppConfig, mt.appConfig, mt.invalid) require.NoError(t, err) checkExpectedCAValidationError(t, mt.invalid, status, "integrations.digicert.name", - "name is already used by another DigiCert certificate authority") + "name is already used by another certificate authority") + }) + + t.Run("duplicate custom_scep integration name", func(t *testing.T) { + mt := setUpCustomSCEP() + mt.newAppConfig.Integrations.CustomSCEPProxy.Value = append(mt.newAppConfig.Integrations.CustomSCEPProxy.Value, + mt.newAppConfig.Integrations.CustomSCEPProxy.Value[0]) + status, err := mt.svc.processAppConfigCAs(mt.ctx, mt.newAppConfig, mt.oldAppConfig, mt.appConfig, mt.invalid) + require.NoError(t, err) + checkExpectedCAValidationError(t, mt.invalid, status, "integrations.custom_scep_proxy.name", + "name is already used by another certificate authority") + }) + + t.Run("same digicert and custom_scep integration name", func(t *testing.T) { + mtSCEP := setUpCustomSCEP() + mt := setUpDigiCert() + mt.newAppConfig.Integrations.CustomSCEPProxy = mtSCEP.newAppConfig.Integrations.CustomSCEPProxy + mt.newAppConfig.Integrations.CustomSCEPProxy.Value[0].Name = mt.newAppConfig.Integrations.DigiCert.Value[0].Name + status, err := mt.svc.processAppConfigCAs(mt.ctx, mt.newAppConfig, mt.oldAppConfig, mt.appConfig, mt.invalid) + require.NoError(t, err) + checkExpectedCAValidationError(t, mt.invalid, status, "integrations.custom_scep_proxy.name", + "name is already used by another certificate authority") }) t.Run("digicert more than 1 user principal name", func(t *testing.T) { - mt := setUp() + mt := setUpDigiCert() mt.newAppConfig.Integrations.DigiCert.Value[0].CertificateUserPrincipalNames = append(mt.newAppConfig.Integrations.DigiCert.Value[0].CertificateUserPrincipalNames, "another") status, err := mt.svc.processAppConfigCAs(mt.ctx, mt.newAppConfig, mt.oldAppConfig, mt.appConfig, mt.invalid) @@ -1932,7 +1978,7 @@ func TestAppConfigCAs(t *testing.T) { }) t.Run("digicert Fleet vars in user principal name", func(t *testing.T) { - mt := setUp() + mt := setUpDigiCert() mt.newAppConfig.Integrations.DigiCert.Value[0].CertificateUserPrincipalNames[0] = "$FLEET_VAR_" + FleetVarHostEndUserEmailIDP + " ${FLEET_VAR_" + FleetVarHostHardwareSerial + "}" _, err := mt.svc.processAppConfigCAs(mt.ctx, mt.newAppConfig, mt.oldAppConfig, mt.appConfig, mt.invalid) require.NoError(t, err) @@ -1946,7 +1992,7 @@ func TestAppConfigCAs(t *testing.T) { }) t.Run("digicert Fleet vars in common name", func(t *testing.T) { - mt := setUp() + mt := setUpDigiCert() mt.newAppConfig.Integrations.DigiCert.Value[0].CertificateCommonName = "${FLEET_VAR_" + FleetVarHostEndUserEmailIDP + "}${FLEET_VAR_" + FleetVarHostHardwareSerial + "}" _, err := mt.svc.processAppConfigCAs(mt.ctx, mt.newAppConfig, mt.oldAppConfig, mt.appConfig, mt.invalid) require.NoError(t, err) @@ -1960,7 +2006,7 @@ func TestAppConfigCAs(t *testing.T) { }) t.Run("digicert Fleet vars in seat id", func(t *testing.T) { - mt := setUp() + mt := setUpDigiCert() mt.newAppConfig.Integrations.DigiCert.Value[0].CertificateSeatID = "$FLEET_VAR_" + FleetVarHostEndUserEmailIDP + " $FLEET_VAR_" + FleetVarHostHardwareSerial _, err := mt.svc.processAppConfigCAs(mt.ctx, mt.newAppConfig, mt.oldAppConfig, mt.appConfig, mt.invalid) require.NoError(t, err) @@ -1974,15 +2020,23 @@ func TestAppConfigCAs(t *testing.T) { }) t.Run("digicert API token not set", func(t *testing.T) { - mt := setUp() + mt := setUpDigiCert() mt.newAppConfig.Integrations.DigiCert.Value[0].APIToken = fleet.MaskedPassword status, err := mt.svc.processAppConfigCAs(mt.ctx, mt.newAppConfig, mt.oldAppConfig, mt.appConfig, mt.invalid) require.NoError(t, err) checkExpectedCAValidationError(t, mt.invalid, status, "integrations.digicert.api_token", "DigiCert API token must be set") }) + t.Run("custom_scep challenge not set", func(t *testing.T) { + mt := setUpCustomSCEP() + mt.newAppConfig.Integrations.CustomSCEPProxy.Value[0].Challenge = fleet.MaskedPassword + status, err := mt.svc.processAppConfigCAs(mt.ctx, mt.newAppConfig, mt.oldAppConfig, mt.appConfig, mt.invalid) + require.NoError(t, err) + checkExpectedCAValidationError(t, mt.invalid, status, "integrations.custom_scep_proxy.challenge", "Custom SCEP challenge must be set") + }) + t.Run("digicert common name not set", func(t *testing.T) { - mt := setUp() + mt := setUpDigiCert() mt.newAppConfig.Integrations.DigiCert.Value[0].CertificateCommonName = "\n\t" status, err := mt.svc.processAppConfigCAs(mt.ctx, mt.newAppConfig, mt.oldAppConfig, mt.appConfig, mt.invalid) require.NoError(t, err) @@ -1990,7 +2044,7 @@ func TestAppConfigCAs(t *testing.T) { }) t.Run("digicert seat id not set", func(t *testing.T) { - mt := setUp() + mt := setUpDigiCert() mt.newAppConfig.Integrations.DigiCert.Value[0].CertificateSeatID = "\t\n" status, err := mt.svc.processAppConfigCAs(mt.ctx, mt.newAppConfig, mt.oldAppConfig, mt.appConfig, mt.invalid) require.NoError(t, err) @@ -1998,10 +2052,11 @@ func TestAppConfigCAs(t *testing.T) { }) t.Run("digicert happy path -- add one", func(t *testing.T) { - mt := setUp() + mt := setUpDigiCert() status, err := mt.svc.processAppConfigCAs(mt.ctx, mt.newAppConfig, mt.oldAppConfig, mt.appConfig, mt.invalid) require.NoError(t, err) assert.Empty(t, mt.invalid.Errors) + assert.Empty(t, status.customSCEPProxy) require.Len(t, status.digicert, 1) assert.Equal(t, caStatusAdded, status.digicert[mt.newAppConfig.Integrations.DigiCert.Value[0].Name]) require.Len(t, mt.appConfig.Integrations.DigiCert.Value, 1) @@ -2009,7 +2064,7 @@ func TestAppConfigCAs(t *testing.T) { }) t.Run("digicert happy path -- delete one", func(t *testing.T) { - mt := setUp() + mt := setUpDigiCert() mt.oldAppConfig = mt.newAppConfig mt.appConfig = mt.oldAppConfig.Copy() mt.newAppConfig = &fleet.AppConfig{ @@ -2023,13 +2078,14 @@ func TestAppConfigCAs(t *testing.T) { status, err := mt.svc.processAppConfigCAs(mt.ctx, mt.newAppConfig, mt.oldAppConfig, mt.appConfig, mt.invalid) require.NoError(t, err) assert.Empty(t, mt.invalid.Errors) + assert.Empty(t, status.customSCEPProxy) require.Len(t, status.digicert, 1) assert.Equal(t, caStatusDeleted, status.digicert[mt.oldAppConfig.Integrations.DigiCert.Value[0].Name]) assert.False(t, mt.appConfig.Integrations.DigiCert.Valid) }) t.Run("digicert API token not set on modify", func(t *testing.T) { - mt := setUp() + mt := setUpDigiCert() mt.oldAppConfig.Integrations.DigiCert.Value = append(mt.oldAppConfig.Integrations.DigiCert.Value, mt.newAppConfig.Integrations.DigiCert.Value[0]) mt.appConfig = mt.oldAppConfig.Copy() @@ -2041,7 +2097,7 @@ func TestAppConfigCAs(t *testing.T) { }) t.Run("digicert happy path -- add one, delete one, modify one", func(t *testing.T) { - mt := setUp() + mt := setUpDigiCert() mt.newAppConfig.Integrations.DigiCert = optjson.Slice[fleet.DigiCertIntegration]{ Set: true, Valid: true, @@ -2112,6 +2168,7 @@ func TestAppConfigCAs(t *testing.T) { status, err := mt.svc.processAppConfigCAs(mt.ctx, mt.newAppConfig, mt.oldAppConfig, mt.appConfig, mt.invalid) require.NoError(t, err) assert.Empty(t, mt.invalid.Errors) + assert.Empty(t, status.customSCEPProxy) require.Len(t, status.digicert, 3) assert.Equal(t, caStatusAdded, status.digicert["add"]) assert.Equal(t, caStatusEdited, status.digicert["modify"]) @@ -2119,6 +2176,145 @@ func TestAppConfigCAs(t *testing.T) { require.Len(t, mt.appConfig.Integrations.DigiCert.Value, 3) }) + t.Run("custom_scep happy path -- add one", func(t *testing.T) { + mt := setUpCustomSCEP() + status, err := mt.svc.processAppConfigCAs(mt.ctx, mt.newAppConfig, mt.oldAppConfig, mt.appConfig, mt.invalid) + require.NoError(t, err) + assert.Empty(t, mt.invalid.Errors) + assert.Empty(t, status.digicert) + require.Len(t, status.customSCEPProxy, 1) + assert.Equal(t, caStatusAdded, status.customSCEPProxy[mt.newAppConfig.Integrations.CustomSCEPProxy.Value[0].Name]) + require.Len(t, mt.appConfig.Integrations.CustomSCEPProxy.Value, 1) + assert.True(t, mt.newAppConfig.Integrations.CustomSCEPProxy.Value[0].Equals(&mt.appConfig.Integrations.CustomSCEPProxy.Value[0])) + }) + + t.Run("custom_scep happy path -- delete one", func(t *testing.T) { + mt := setUpCustomSCEP() + mt.oldAppConfig = mt.newAppConfig + mt.appConfig = mt.oldAppConfig.Copy() + mt.newAppConfig = &fleet.AppConfig{ + Integrations: fleet.Integrations{ + CustomSCEPProxy: optjson.Slice[fleet.CustomSCEPProxyIntegration]{ + Set: true, + Valid: true, + }, + }, + } + status, err := mt.svc.processAppConfigCAs(mt.ctx, mt.newAppConfig, mt.oldAppConfig, mt.appConfig, mt.invalid) + require.NoError(t, err) + assert.Empty(t, mt.invalid.Errors) + assert.Empty(t, status.digicert) + require.Len(t, status.customSCEPProxy, 1) + assert.Equal(t, caStatusDeleted, status.customSCEPProxy[mt.oldAppConfig.Integrations.CustomSCEPProxy.Value[0].Name]) + assert.False(t, mt.appConfig.Integrations.CustomSCEPProxy.Valid) + }) + + t.Run("custom_scep API token not set on modify", func(t *testing.T) { + mt := setUpCustomSCEP() + mt.oldAppConfig.Integrations.CustomSCEPProxy.Value = append(mt.oldAppConfig.Integrations.CustomSCEPProxy.Value, + mt.newAppConfig.Integrations.CustomSCEPProxy.Value[0]) + mt.appConfig = mt.oldAppConfig.Copy() + mt.newAppConfig.Integrations.CustomSCEPProxy.Value[0].URL = "https://new.com" + mt.newAppConfig.Integrations.CustomSCEPProxy.Value[0].Challenge = "" + status, err := mt.svc.processAppConfigCAs(mt.ctx, mt.newAppConfig, mt.oldAppConfig, mt.appConfig, mt.invalid) + require.NoError(t, err) + checkExpectedCAValidationError(t, mt.invalid, status, "integrations.custom_scep_proxy.challenge", + "Custom SCEP challenge must be set when modifying") + }) + + t.Run("custom_scep happy path -- add one, delete one, modify one", func(t *testing.T) { + mt := setUpCustomSCEP() + mt.newAppConfig.Integrations.CustomSCEPProxy = optjson.Slice[fleet.CustomSCEPProxyIntegration]{ + Set: true, + Valid: true, + Value: []fleet.CustomSCEPProxyIntegration{ + { + Name: "add", + URL: "https://example.com", + Challenge: "challenge", + }, + { + Name: "modify", + URL: "https://example.com", + Challenge: "challenge", + }, + { + Name: "SCEP_WIFI", // same + URL: "https://example.com", + Challenge: "challenge", + }, + }, + } + mt.oldAppConfig.Integrations.CustomSCEPProxy = optjson.Slice[fleet.CustomSCEPProxyIntegration]{ + Set: true, + Valid: true, + Value: []fleet.CustomSCEPProxyIntegration{ + { + Name: "delete", + URL: "https://example.com", + Challenge: "challenge", + }, + { + Name: "modify", + URL: "https://modify.com", + Challenge: "challenge", + }, + { + Name: "SCEP_WIFI", // same + URL: "https://example.com", + Challenge: fleet.MaskedPassword, + }, + }, + } + mt.appConfig = mt.oldAppConfig.Copy() + status, err := mt.svc.processAppConfigCAs(mt.ctx, mt.newAppConfig, mt.oldAppConfig, mt.appConfig, mt.invalid) + require.NoError(t, err) + assert.Empty(t, mt.invalid.Errors) + assert.Empty(t, status.digicert) + require.Len(t, status.customSCEPProxy, 3) + assert.Equal(t, caStatusAdded, status.customSCEPProxy["add"]) + assert.Equal(t, caStatusEdited, status.customSCEPProxy["modify"]) + assert.Equal(t, caStatusDeleted, status.customSCEPProxy["delete"]) + require.Len(t, mt.appConfig.Integrations.CustomSCEPProxy.Value, 3) + }) + +} + +type configCASuite struct { + ctx context.Context + svc *Service + appConfig *fleet.AppConfig + newAppConfig *fleet.AppConfig + oldAppConfig *fleet.AppConfig + invalid *fleet.InvalidArgumentError +} + +func addMockDatastoreForCA(t *testing.T, s configCASuite) { + mockDS := &mock.Store{} + s.svc.ds = mockDS + mockDS.GetAllCAConfigAssetsByTypeFunc = func(ctx context.Context, assetType fleet.CAConfigAssetType) (map[string]fleet.CAConfigAsset, error) { + switch assetType { + case fleet.CAConfigDigiCert: + return map[string]fleet.CAConfigAsset{ + "WIFI": { + Name: "WIFI", + Value: []byte("api_token"), + Type: fleet.CAConfigDigiCert, + }, + }, nil + case fleet.CAConfigCustomSCEPProxy: + return map[string]fleet.CAConfigAsset{ + "SCEP_WIFI": { + Name: "SCEP_WIFI", + Value: []byte("challenge"), + Type: fleet.CAConfigCustomSCEPProxy, + }, + }, nil + default: + t.Fatalf("unexpected asset type: %s", assetType) + } + return nil, nil + } } func checkExpectedCAValidationError(t *testing.T, invalid *fleet.InvalidArgumentError, status appConfigCAStatus, contains ...string) { @@ -2156,3 +2352,25 @@ func getDigiCertIntegration(url string, name string) fleet.DigiCertIntegration { } return digiCertCA } + +func getAppConfigWithSCEPIntegration(url string, name string) *fleet.AppConfig { + newAppConfig := &fleet.AppConfig{ + Integrations: fleet.Integrations{ + CustomSCEPProxy: optjson.Slice[fleet.CustomSCEPProxyIntegration]{ + Set: true, + Valid: true, + Value: []fleet.CustomSCEPProxyIntegration{getCustomSCEPIntegration(url, name)}, + }, + }, + } + return newAppConfig +} + +func getCustomSCEPIntegration(url string, name string) fleet.CustomSCEPProxyIntegration { + challenge, _ := server.GenerateRandomText(6) + return fleet.CustomSCEPProxyIntegration{ + Name: name, + URL: url, + Challenge: challenge, + } +} diff --git a/server/service/apple_mdm.go b/server/service/apple_mdm.go index b5026b3304..f86bd542e7 100644 --- a/server/service/apple_mdm.go +++ b/server/service/apple_mdm.go @@ -94,9 +94,6 @@ type hostProfileUUID struct { ProfileUUID string } -// Functions that can be overwritten in tests -var getNDESSCEPChallenge = eeservice.GetNDESSCEPChallenge - type getMDMAppleCommandResultsRequest struct { CommandUUID string `query:"command_uuid,optional"` } @@ -3648,7 +3645,8 @@ func ReconcileAppleProfiles( } // Insert variables into profile contents of install targets. Variables may be host-specific. - err = preprocessProfileContents(ctx, appConfig, ds, logger, installTargets, profileContents, hostProfilesToInstallMap) + err = preprocessProfileContents(ctx, appConfig, ds, eeservice.NewSCEPConfigService(logger, nil), logger, installTargets, profileContents, + hostProfilesToInstallMap) if err != nil { return err } @@ -3767,6 +3765,7 @@ func preprocessProfileContents( ctx context.Context, appConfig *fleet.AppConfig, ds fleet.Datastore, + scepConfig fleet.SCEPConfigService, logger kitlog.Logger, targets map[string]*cmdTarget, profileContents map[string]mobileconfig.Mobileconfig, @@ -3893,7 +3892,7 @@ func preprocessProfileContents( ndesConfig = &configWithPassword } // Insert the SCEP challenge into the profile contents - challenge, err := getNDESSCEPChallenge(ctx, *ndesConfig) + challenge, err := scepConfig.GetNDESSCEPChallenge(ctx, *ndesConfig) if err != nil { detail := "" switch { diff --git a/server/service/apple_mdm_test.go b/server/service/apple_mdm_test.go index 2cafa891c7..ade540a06a 100644 --- a/server/service/apple_mdm_test.go +++ b/server/service/apple_mdm_test.go @@ -44,6 +44,7 @@ import ( "github.com/fleetdm/fleet/v4/server/mock" mdmmock "github.com/fleetdm/fleet/v4/server/mock/mdm" nanodep_mock "github.com/fleetdm/fleet/v4/server/mock/nanodep" + scep_mock "github.com/fleetdm/fleet/v4/server/mock/scep" "github.com/fleetdm/fleet/v4/server/ptr" "github.com/fleetdm/fleet/v4/server/test" kitlog "github.com/go-kit/log" @@ -2792,11 +2793,6 @@ func TestMDMAppleReconcileAppleProfiles(t *testing.T) { } func TestPreprocessProfileContents(t *testing.T) { - origGetNDESSCEPChallenge := getNDESSCEPChallenge - t.Cleanup(func() { - getNDESSCEPChallenge = origGetNDESSCEPChallenge - }) - ctx := context.Background() logger := kitlog.NewNopLogger() appCfg := &fleet.AppConfig{} @@ -2806,7 +2802,8 @@ func TestPreprocessProfileContents(t *testing.T) { ds := new(mock.Store) // No-op - err := preprocessProfileContents(ctx, appCfg, ds, logger, nil, nil, nil) + svc := eeservice.NewSCEPConfigService(logger, nil) + err := preprocessProfileContents(ctx, appCfg, ds, svc, logger, nil, nil, nil) require.NoError(t, err) hostUUID := "host-1" @@ -2846,7 +2843,7 @@ func TestPreprocessProfileContents(t *testing.T) { } // Can't use NDES SCEP proxy with free tier ctx = license.NewContext(ctx, &fleet.LicenseInfo{Tier: fleet.TierFree}) - err = preprocessProfileContents(ctx, appCfg, ds, logger, targets, profileContents, hostProfilesToInstallMap) + err = preprocessProfileContents(ctx, appCfg, ds, svc, logger, targets, profileContents, hostProfilesToInstallMap) require.NoError(t, err) require.NotNil(t, updatedPayload) assert.Contains(t, updatedPayload.Detail, "Premium license") @@ -2857,7 +2854,7 @@ func TestPreprocessProfileContents(t *testing.T) { appCfg.Integrations.NDESSCEPProxy.Valid = false updatedPayload = nil populateTargets() - err = preprocessProfileContents(ctx, appCfg, ds, logger, targets, profileContents, hostProfilesToInstallMap) + err = preprocessProfileContents(ctx, appCfg, ds, svc, logger, targets, profileContents, hostProfilesToInstallMap) require.NoError(t, err) require.NotNil(t, updatedPayload) assert.Contains(t, updatedPayload.Detail, "not configured") @@ -2870,7 +2867,7 @@ func TestPreprocessProfileContents(t *testing.T) { appCfg.Integrations.NDESSCEPProxy.Valid = true updatedPayload = nil populateTargets() - err = preprocessProfileContents(ctx, appCfg, ds, logger, targets, profileContents, hostProfilesToInstallMap) + err = preprocessProfileContents(ctx, appCfg, ds, svc, logger, targets, profileContents, hostProfilesToInstallMap) require.NoError(t, err) require.NotNil(t, updatedPayload) assert.Contains(t, updatedPayload.Detail, "FLEET_VAR_BOZO") @@ -2905,7 +2902,8 @@ func TestPreprocessProfileContents(t *testing.T) { profileContents = map[string]mobileconfig.Mobileconfig{ "p1": []byte("$FLEET_VAR_" + FleetVarNDESSCEPChallenge), } - getNDESSCEPChallenge = func(ctx context.Context, proxy fleet.NDESSCEPProxyIntegration) (string, error) { + scepConfig := &scep_mock.SCEPConfigService{} + scepConfig.GetNDESSCEPChallengeFunc = func(ctx context.Context, proxy fleet.NDESSCEPProxyIntegration) (string, error) { assert.Equal(t, ndesPassword, proxy.Password) return "", eeservice.NewNDESInvalidError("NDES error") } @@ -2915,7 +2913,7 @@ func TestPreprocessProfileContents(t *testing.T) { assert.Empty(t, payload) // no profiles to update since FLEET VAR could not be populated return nil } - err = preprocessProfileContents(ctx, appCfg, ds, logger, targets, profileContents, hostProfilesToInstallMap) + err = preprocessProfileContents(ctx, appCfg, ds, scepConfig, logger, targets, profileContents, hostProfilesToInstallMap) require.NoError(t, err) require.NotNil(t, updatedProfile) assert.Contains(t, updatedProfile.Detail, "FLEET_VAR_"+FleetVarNDESSCEPChallenge) @@ -2923,13 +2921,13 @@ func TestPreprocessProfileContents(t *testing.T) { assert.Empty(t, targets) // Password cache full - getNDESSCEPChallenge = func(ctx context.Context, proxy fleet.NDESSCEPProxyIntegration) (string, error) { + scepConfig.GetNDESSCEPChallengeFunc = func(ctx context.Context, proxy fleet.NDESSCEPProxyIntegration) (string, error) { assert.Equal(t, ndesPassword, proxy.Password) return "", eeservice.NewNDESPasswordCacheFullError("NDES error") } updatedProfile = nil populateTargets() - err = preprocessProfileContents(ctx, appCfg, ds, logger, targets, profileContents, hostProfilesToInstallMap) + err = preprocessProfileContents(ctx, appCfg, ds, scepConfig, logger, targets, profileContents, hostProfilesToInstallMap) require.NoError(t, err) require.NotNil(t, updatedProfile) assert.Contains(t, updatedProfile.Detail, "FLEET_VAR_"+FleetVarNDESSCEPChallenge) @@ -2937,13 +2935,13 @@ func TestPreprocessProfileContents(t *testing.T) { assert.Empty(t, targets) // Insufficient permissions - getNDESSCEPChallenge = func(ctx context.Context, proxy fleet.NDESSCEPProxyIntegration) (string, error) { + scepConfig.GetNDESSCEPChallengeFunc = func(ctx context.Context, proxy fleet.NDESSCEPProxyIntegration) (string, error) { assert.Equal(t, ndesPassword, proxy.Password) return "", eeservice.NewNDESInsufficientPermissionsError("NDES error") } updatedProfile = nil populateTargets() - err = preprocessProfileContents(ctx, appCfg, ds, logger, targets, profileContents, hostProfilesToInstallMap) + err = preprocessProfileContents(ctx, appCfg, ds, scepConfig, logger, targets, profileContents, hostProfilesToInstallMap) require.NoError(t, err) require.NotNil(t, updatedProfile) assert.Contains(t, updatedProfile.Detail, "FLEET_VAR_"+FleetVarNDESSCEPChallenge) @@ -2951,13 +2949,13 @@ func TestPreprocessProfileContents(t *testing.T) { assert.Empty(t, targets) // Other NDES challenge error - getNDESSCEPChallenge = func(ctx context.Context, proxy fleet.NDESSCEPProxyIntegration) (string, error) { + scepConfig.GetNDESSCEPChallengeFunc = func(ctx context.Context, proxy fleet.NDESSCEPProxyIntegration) (string, error) { assert.Equal(t, ndesPassword, proxy.Password) return "", errors.New("NDES error") } updatedProfile = nil populateTargets() - err = preprocessProfileContents(ctx, appCfg, ds, logger, targets, profileContents, hostProfilesToInstallMap) + err = preprocessProfileContents(ctx, appCfg, ds, scepConfig, logger, targets, profileContents, hostProfilesToInstallMap) require.NoError(t, err) require.NotNil(t, updatedProfile) assert.Contains(t, updatedProfile.Detail, "FLEET_VAR_"+FleetVarNDESSCEPChallenge) @@ -2967,7 +2965,7 @@ func TestPreprocessProfileContents(t *testing.T) { // NDES challenge challenge := "ndes-challenge" - getNDESSCEPChallenge = func(ctx context.Context, proxy fleet.NDESSCEPProxyIntegration) (string, error) { + scepConfig.GetNDESSCEPChallengeFunc = func(ctx context.Context, proxy fleet.NDESSCEPProxyIntegration) (string, error) { assert.Equal(t, ndesPassword, proxy.Password) return challenge, nil } @@ -2984,7 +2982,7 @@ func TestPreprocessProfileContents(t *testing.T) { assert.NotNil(t, payload[0].ChallengeRetrievedAt) return nil } - err = preprocessProfileContents(ctx, appCfg, ds, logger, targets, profileContents, hostProfilesToInstallMap) + err = preprocessProfileContents(ctx, appCfg, ds, scepConfig, logger, targets, profileContents, hostProfilesToInstallMap) require.NoError(t, err) assert.Nil(t, updatedProfile) require.NotEmpty(t, targets) @@ -3007,7 +3005,7 @@ func TestPreprocessProfileContents(t *testing.T) { assert.Empty(t, payload) return nil } - err = preprocessProfileContents(ctx, appCfg, ds, logger, targets, profileContents, hostProfilesToInstallMap) + err = preprocessProfileContents(ctx, appCfg, ds, scepConfig, logger, targets, profileContents, hostProfilesToInstallMap) require.NoError(t, err) assert.Nil(t, updatedProfile) require.NotEmpty(t, targets) @@ -3028,7 +3026,7 @@ func TestPreprocessProfileContents(t *testing.T) { } updatedProfile = nil populateTargets() - err = preprocessProfileContents(ctx, appCfg, ds, logger, targets, profileContents, hostProfilesToInstallMap) + err = preprocessProfileContents(ctx, appCfg, ds, scepConfig, logger, targets, profileContents, hostProfilesToInstallMap) require.NoError(t, err) require.NotNil(t, updatedProfile) assert.Contains(t, updatedProfile.Detail, "FLEET_VAR_"+FleetVarHostEndUserEmailIDP) @@ -3042,7 +3040,7 @@ func TestPreprocessProfileContents(t *testing.T) { } updatedProfile = nil populateTargets() - err = preprocessProfileContents(ctx, appCfg, ds, logger, targets, profileContents, hostProfilesToInstallMap) + err = preprocessProfileContents(ctx, appCfg, ds, scepConfig, logger, targets, profileContents, hostProfilesToInstallMap) require.NoError(t, err) assert.Nil(t, updatedProfile) require.NotEmpty(t, targets) @@ -3066,7 +3064,7 @@ func TestPreprocessProfileContents(t *testing.T) { } updatedProfile = nil populateTargets() - err = preprocessProfileContents(ctx, appCfg, ds, logger, targets, profileContents, hostProfilesToInstallMap) + err = preprocessProfileContents(ctx, appCfg, ds, scepConfig, logger, targets, profileContents, hostProfilesToInstallMap) require.NoError(t, err) assert.Nil(t, updatedProfile) require.NotEmpty(t, targets) @@ -3085,7 +3083,7 @@ func TestPreprocessProfileContents(t *testing.T) { } updatedProfile = nil populateTargets() - err = preprocessProfileContents(ctx, appCfg, ds, logger, targets, profileContents, hostProfilesToInstallMap) + err = preprocessProfileContents(ctx, appCfg, ds, scepConfig, logger, targets, profileContents, hostProfilesToInstallMap) require.NoError(t, err) require.NotNil(t, updatedProfile) assert.Contains(t, updatedProfile.Detail, "Unexpected number of hosts (0) for UUID") @@ -3147,7 +3145,7 @@ func TestPreprocessProfileContents(t *testing.T) { } return nil } - err = preprocessProfileContents(ctx, appCfg, ds, logger, targets, profileContents, hostProfilesToInstallMap) + err = preprocessProfileContents(ctx, appCfg, ds, scepConfig, logger, targets, profileContents, hostProfilesToInstallMap) require.NoError(t, err) require.NotEmpty(t, targets) assert.Len(t, targets, 3) diff --git a/server/service/handler.go b/server/service/handler.go index f89c497e0d..5483e288bf 100644 --- a/server/service/handler.go +++ b/server/service/handler.go @@ -8,6 +8,7 @@ import ( "os" "regexp" "strings" + "time" eeservice "github.com/fleetdm/fleet/v4/ee/server/service" "github.com/fleetdm/fleet/v4/server/config" @@ -1117,10 +1118,12 @@ func RegisterSCEPProxy( rootMux *http.ServeMux, ds fleet.Datastore, logger kitlog.Logger, + timeout *time.Duration, ) error { scepService := eeservice.NewSCEPProxyService( ds, kitlog.With(logger, "component", "scep-proxy-service"), + timeout, ) scepLogger := kitlog.With(logger, "component", "http-scep-proxy") e := scepserver.MakeServerEndpointsWithIdentifier(scepService) diff --git a/server/service/integration_mdm_test.go b/server/service/integration_mdm_test.go index 0e9a16e442..ee2c1e6f9b 100644 --- a/server/service/integration_mdm_test.go +++ b/server/service/integration_mdm_test.go @@ -112,6 +112,7 @@ type integrationMDMTestSuite struct { appleITunesSrvData map[string]string appleGDMFSrv *httptest.Server mockedDownloadFleetdmMeta fleetdbase.Metadata + scepConfig *eeservice.SCEPConfigService } // appleVPPConfigSrvConf is used to configure the mock server that mocks Apple's VPP endpoints. @@ -209,6 +210,8 @@ func (s *integrationMDMTestSuite) SetupSuite() { softwareInstallerStore = s3.SetupTestSoftwareInstallerStore(s.T(), "integration-tests", "") bootstrapPackageStore = s3.SetupTestBootstrapPackageStore(s.T(), "integration-tests", "") } + scepTimeout := ptr.Duration(10 * time.Second) + s.scepConfig = eeservice.NewSCEPConfigService(serverLogger, scepTimeout).(*eeservice.SCEPConfigService) serverConfig := TestServerOpts{ License: &fleet.LicenseInfo{ @@ -302,9 +305,10 @@ func (s *integrationMDMTestSuite) SetupSuite() { } }, }, - APNSTopic: "com.apple.mgmt.External.10ac3ce5-4668-4e58-b69a-b2b5ce667589", - EnableSCEPProxy: true, - WithDEPWebview: true, + APNSTopic: "com.apple.mgmt.External.10ac3ce5-4668-4e58-b69a-b2b5ce667589", + EnableSCEPProxy: true, + WithDEPWebview: true, + SCEPConfigService: s.scepConfig, } // ensure all our tests support challenges with invalid XML characters @@ -13098,10 +13102,9 @@ func (s *integrationMDMTestSuite) TestSCEPProxy() { time.Sleep(1 * time.Second) w.WriteHeader(http.StatusOK) })) - origNDESTimeout := eeservice.NDESTimeout - eeservice.NDESTimeout = ptr.Duration(1 * time.Microsecond) + *s.scepConfig.Timeout = time.Microsecond t.Cleanup(func() { - eeservice.NDESTimeout = origNDESTimeout + *s.scepConfig.Timeout = 10 * time.Second ndesTimeoutServer.Close() }) appConf.Integrations.NDESSCEPProxy.Value.URL = ndesTimeoutServer.URL @@ -13114,46 +13117,9 @@ func (s *integrationMDMTestSuite) TestSCEPProxy() { // PKIOperation _ = s.DoRawWithHeaders("GET", apple_mdm.SCEPProxyPath+identifier, nil, http.StatusRequestTimeout, nil, "operation", "PKIOperation", "message", message) - eeservice.NDESTimeout = origNDESTimeout + *s.scepConfig.Timeout = 10 * time.Second - // Spin up an "external" SCEP server, which Fleet server will proxy - newSCEPServer := func(t *testing.T, opts ...scepserver.ServiceOption) *httptest.Server { - var server *httptest.Server - teardown := func() { - if server != nil { - server.Close() - } - os.Remove("./testdata/externalCA/serial") - os.Remove("./testdata/externalCA/index.txt") - } - t.Cleanup(teardown) - - var err error - var certDepot depot.Depot // cert storage - certDepot, err = filedepot.NewFileDepot("./testdata/externalCA") - if err != nil { - t.Fatal(err) - } - certDepot = &noopCertDepot{certDepot} - crt, key, err := certDepot.CA([]byte{}) - if err != nil { - t.Fatal(err) - } - - var svc scepserver.Service // scep service - svc, err = scepserver.NewService(crt[0], key, scepserver.NopCSRSigner()) - if err != nil { - t.Fatal(err) - } - logger := kitlog.NewNopLogger() - e := scepserver.MakeServerEndpoints(svc) - scepHandler := scepserver.MakeHTTPHandler(e, svc, logger) - r := mux.NewRouter() - r.Handle("/scep", scepHandler) - server = httptest.NewServer(r) - return server - } - scepServer := newSCEPServer(t) + scepServer := startSCEPServer(t) appConf.Integrations.NDESSCEPProxy.Value.URL = scepServer.URL + "/scep" err = s.ds.SaveAppConfig(context.Background(), appConf) @@ -13254,6 +13220,48 @@ func (s *integrationMDMTestSuite) TestSCEPProxy() { assert.Equal(t, scep.CertRep, pkiMessage.MessageType) } +func startSCEPServer(t *testing.T) *httptest.Server { + // Spin up an "external" SCEP server, which Fleet server will proxy + newSCEPServer := func(t *testing.T) *httptest.Server { + var server *httptest.Server + teardown := func() { + if server != nil { + server.Close() + } + os.Remove("./testdata/externalCA/serial") + os.Remove("./testdata/externalCA/index.txt") + } + t.Cleanup(teardown) + + var err error + var certDepot depot.Depot // cert storage + certDepot, err = filedepot.NewFileDepot("./testdata/externalCA") + if err != nil { + t.Fatal(err) + } + certDepot = &noopCertDepot{certDepot} + crt, key, err := certDepot.CA([]byte{}) + if err != nil { + t.Fatal(err) + } + + var svc scepserver.Service // scep service + svc, err = scepserver.NewService(crt[0], key, scepserver.NopCSRSigner()) + if err != nil { + t.Fatal(err) + } + logger := kitlog.NewNopLogger() + e := scepserver.MakeServerEndpoints(svc) + scepHandler := scepserver.MakeHTTPHandler(e, svc, logger) + r := mux.NewRouter() + r.Handle("/scep", scepHandler) + server = httptest.NewServer(r) + return server + } + scepServer := newSCEPServer(t) + return scepServer +} + func (s *integrationMDMTestSuite) TestDigiCertConfig() { t := s.T() ctx := context.Background() @@ -13273,7 +13281,7 @@ func (s *integrationMDMTestSuite) TestDigiCertConfig() { rawRes := s.Do("PATCH", "/api/latest/fleet/config", &req, http.StatusUnprocessableEntity, "dry_run", "true") errMsg := extractServerErrorText(rawRes.Body) require.Contains(t, errMsg, "Could not verify DigiCert profile ID") - _, err = s.ds.GetAllCAConfigAssets(ctx) + _, err = s.ds.GetAllCAConfigAssetsByType(ctx, fleet.CAConfigDigiCert) assert.True(t, fleet.IsNotFound(err)) // Add 3 DigiCert integrations @@ -13295,7 +13303,7 @@ func (s *integrationMDMTestSuite) TestDigiCertConfig() { res := appConfigResponse{} s.DoJSON("PATCH", "/api/latest/fleet/config", &req, http.StatusOK, &res, "dry_run", "true") assert.Empty(t, res.Integrations.DigiCert.Value) - _, err = s.ds.GetAllCAConfigAssets(ctx) + _, err = s.ds.GetAllCAConfigAssetsByType(ctx, fleet.CAConfigDigiCert) assert.True(t, fleet.IsNotFound(err)) res = appConfigResponse{} @@ -13304,7 +13312,7 @@ func (s *integrationMDMTestSuite) TestDigiCertConfig() { for _, ca := range res.Integrations.DigiCert.Value { assert.Equal(t, ca.APIToken, fleet.MaskedPassword) } - assets, err := s.ds.GetAllCAConfigAssets(ctx) + assets, err := s.ds.GetAllCAConfigAssetsByType(ctx, fleet.CAConfigDigiCert) require.NoError(t, err) require.Len(t, assets, 3) assert.EqualValues(t, "api_token0", assets["ca0"].Value) @@ -13361,7 +13369,7 @@ func (s *integrationMDMTestSuite) TestDigiCertConfig() { } assert.Equal(t, ca.APIToken, fleet.MaskedPassword) } - assets, err = s.ds.GetAllCAConfigAssets(ctx) + assets, err = s.ds.GetAllCAConfigAssetsByType(ctx, fleet.CAConfigDigiCert) require.NoError(t, err) require.Len(t, assets, 3) assert.EqualValues(t, "api_token1", assets["ca1"].Value) @@ -13412,7 +13420,7 @@ func (s *integrationMDMTestSuite) TestDigiCertConfig() { res = appConfigResponse{} s.DoJSON("PATCH", "/api/latest/fleet/config", &req, http.StatusOK, &res) assert.Empty(t, res.Integrations.DigiCert.Value) - _, err = s.ds.GetAllCAConfigAssets(ctx) + _, err = s.ds.GetAllCAConfigAssetsByType(ctx, fleet.CAConfigDigiCert) assert.True(t, fleet.IsNotFound(err)) // Check that 3 deleted activities are present @@ -13890,6 +13898,189 @@ func createMockDigiCertServer(t *testing.T) *mockDigiCertServer { return mockServer } +func (s *integrationMDMTestSuite) TestCustomSCEPConfig() { + t := s.T() + ctx := context.Background() + scepServer := startSCEPServer(t) + scepServerURL := scepServer.URL + "/scep" + + // Add custom SCEP integration with bad URL + caBad := getCustomSCEPIntegration("https://httpstat.us/410", "ca") + appConfig := map[string]interface{}{ + "integrations": map[string]interface{}{ + "custom_scep_proxy": []fleet.CustomSCEPProxyIntegration{caBad}, + }, + } + raw, err := json.Marshal(appConfig) + require.NoError(t, err) + var req modifyAppConfigRequest + req.RawMessage = raw + rawRes := s.Do("PATCH", "/api/latest/fleet/config", &req, http.StatusUnprocessableEntity, "dry_run", "true") + errMsg := extractServerErrorText(rawRes.Body) + assert.Contains(t, errMsg, "invalid SCEP URL") + _, err = s.ds.GetAllCAConfigAssetsByType(ctx, fleet.CAConfigCustomSCEPProxy) + assert.True(t, fleet.IsNotFound(err)) + + // Add 3 CustomSCEPProxy integrations + ca0 := getCustomSCEPIntegration(scepServerURL, "ca0") + ca0.Challenge = "challenge0" + ca1 := getCustomSCEPIntegration(scepServerURL, "ca1") + ca1.Challenge = "challenge1" + ca2 := getCustomSCEPIntegration(scepServerURL, "ca2") + ca2.Challenge = "challenge2" + appConfig = map[string]interface{}{ + "integrations": map[string]interface{}{ + "custom_scep_proxy": []fleet.CustomSCEPProxyIntegration{ca0, ca1, ca2}, + }, + } + raw, err = json.Marshal(appConfig) + require.NoError(t, err) + req = modifyAppConfigRequest{} + req.RawMessage = raw + res := appConfigResponse{} + s.DoJSON("PATCH", "/api/latest/fleet/config", &req, http.StatusOK, &res, "dry_run", "true") + assert.Empty(t, res.Integrations.CustomSCEPProxy.Value) + _, err = s.ds.GetAllCAConfigAssetsByType(ctx, fleet.CAConfigCustomSCEPProxy) + assert.True(t, fleet.IsNotFound(err)) + + res = appConfigResponse{} + s.DoJSON("PATCH", "/api/latest/fleet/config", &req, http.StatusOK, &res) + assert.Len(t, res.Integrations.CustomSCEPProxy.Value, 3) + for _, ca := range res.Integrations.CustomSCEPProxy.Value { + assert.Equal(t, fleet.MaskedPassword, ca.Challenge) + } + assets, err := s.ds.GetAllCAConfigAssetsByType(ctx, fleet.CAConfigCustomSCEPProxy) + require.NoError(t, err) + require.Len(t, assets, 3) + assert.EqualValues(t, "challenge0", assets["ca0"].Value) + assert.EqualValues(t, "challenge1", assets["ca1"].Value) + assert.EqualValues(t, "challenge2", assets["ca2"].Value) + + // Check 3 added activities are present + var listActivities listActivitiesResponse + s.DoJSON("GET", "/api/latest/fleet/activities", nil, http.StatusOK, + &listActivities, "order_key", "a.id", "order_direction", "desc", "per_page", "10") + require.True(t, len(listActivities.Activities) > 0) + activity := fleet.ActivityAddedCustomSCEPProxy{} + caNames := make([]string, 0, 3) + for _, act := range listActivities.Activities { + if act.Type == activity.ActivityName() { + err := json.Unmarshal(*act.Details, &activity) + require.NoError(t, err) + caNames = append(caNames, activity.Name) + if len(caNames) == 3 { + break + } + } + } + slices.Sort(caNames) + assert.EqualValues(t, caNames, []string{"ca0", "ca1", "ca2"}) + + // Add 1, modify 1, delete 1, keep 1 the same (CustomSCEPProxy integrations) + ca1.Challenge = "challenge1-modified" + ca3 := getCustomSCEPIntegration(scepServerURL, "ca3") + ca3.Challenge = "challenge3" + appConfig = map[string]interface{}{ + "integrations": map[string]interface{}{ + "custom_scep_proxy": []fleet.CustomSCEPProxyIntegration{ca3, ca2, ca1}, + }, + } + raw, err = json.Marshal(appConfig) + require.NoError(t, err) + req.RawMessage = raw + res = appConfigResponse{} + s.DoJSON("PATCH", "/api/latest/fleet/config", &req, http.StatusOK, &res) + require.Len(t, res.Integrations.CustomSCEPProxy.Value, 3) + assert.NotEqual(t, res.Integrations.CustomSCEPProxy.Value[0].Name, res.Integrations.CustomSCEPProxy.Value[1].Name) + assert.NotEqual(t, res.Integrations.CustomSCEPProxy.Value[1].Name, res.Integrations.CustomSCEPProxy.Value[2].Name) + for _, ca := range res.Integrations.CustomSCEPProxy.Value { + switch ca.Name { + case "ca1": + assert.True(t, ca.Equals(&ca1)) + case "ca2": + assert.True(t, ca.Equals(&ca2)) + case "ca3": + assert.True(t, ca.Equals(&ca3)) + default: + t.Fatalf("unexpected ca name: %s", ca.Name) + } + assert.Equal(t, ca.Challenge, fleet.MaskedPassword) + } + assets, err = s.ds.GetAllCAConfigAssetsByType(ctx, fleet.CAConfigCustomSCEPProxy) + require.NoError(t, err) + require.Len(t, assets, 3) + assert.EqualValues(t, "challenge1-modified", assets["ca1"].Value) + assert.EqualValues(t, "challenge2", assets["ca2"].Value) + assert.EqualValues(t, "challenge3", assets["ca3"].Value) + + listActivities = listActivitiesResponse{} + s.DoJSON("GET", "/api/latest/fleet/activities", nil, http.StatusOK, + &listActivities, "order_key", "a.id", "order_direction", "desc", "per_page", "10") + require.True(t, len(listActivities.Activities) > 0) + activity = fleet.ActivityAddedCustomSCEPProxy{} + editActivity := fleet.ActivityEditedCustomSCEPProxy{} + delActivity := fleet.ActivityDeletedCustomSCEPProxy{} + var numFound int + for _, act := range listActivities.Activities { + switch act.Type { + case activity.ActivityName(): + err := json.Unmarshal(*act.Details, &activity) + require.NoError(t, err) + assert.Equal(t, activity.Name, ca3.Name) + numFound++ + case editActivity.ActivityName(): + err := json.Unmarshal(*act.Details, &editActivity) + require.NoError(t, err) + assert.Equal(t, editActivity.Name, ca1.Name) + numFound++ + case delActivity.ActivityName(): + err := json.Unmarshal(*act.Details, &delActivity) + require.NoError(t, err) + assert.Equal(t, delActivity.Name, ca0.Name) + numFound++ + } + if numFound == 3 { + break + } + } + assert.Equal(t, 3, numFound) + + // Clear CustomSCEPProxy integrations + appConfig = map[string]interface{}{ + "integrations": map[string]interface{}{ + "custom_scep_proxy": nil, + }, + } + raw, err = json.Marshal(appConfig) + require.NoError(t, err) + req.RawMessage = raw + res = appConfigResponse{} + s.DoJSON("PATCH", "/api/latest/fleet/config", &req, http.StatusOK, &res) + assert.Empty(t, res.Integrations.CustomSCEPProxy.Value) + _, err = s.ds.GetAllCAConfigAssetsByType(ctx, fleet.CAConfigCustomSCEPProxy) + assert.True(t, fleet.IsNotFound(err)) + + // Check that 3 deleted activities are present + listActivities = listActivitiesResponse{} + s.DoJSON("GET", "/api/latest/fleet/activities", nil, http.StatusOK, + &listActivities, "order_key", "a.id", "order_direction", "desc", "per_page", "10") + require.True(t, len(listActivities.Activities) > 0) + delActivity = fleet.ActivityDeletedCustomSCEPProxy{} + caNames = make([]string, 0, 3) + for _, act := range listActivities.Activities { + if act.Type == delActivity.ActivityName() { + err := json.Unmarshal(*act.Details, &delActivity) + require.NoError(t, err) + caNames = append(caNames, delActivity.Name) + if len(caNames) == 3 { + break + } + } + } + slices.Sort(caNames) + assert.EqualValues(t, caNames, []string{"ca1", "ca2", "ca3"}) +} + type noopCertDepot struct{ depot.Depot } func (d *noopCertDepot) Put(_ string, _ *x509.Certificate) error { diff --git a/server/service/service.go b/server/service/service.go index 88f0133c93..7d86c8e077 100644 --- a/server/service/service.go +++ b/server/service/service.go @@ -60,7 +60,8 @@ type Service struct { cronSchedulesService fleet.CronSchedulesService - wstepCertManager microsoft_mdm.CertManager + wstepCertManager microsoft_mdm.CertManager + scepConfigService fleet.SCEPConfigService } func (svc *Service) LookupGeoIP(ctx context.Context, ip string) *fleet.GeoLocation { @@ -105,6 +106,7 @@ func NewService( mdmPushService nanomdm_push.Pusher, cronSchedulesService fleet.CronSchedulesService, wstepCertManager microsoft_mdm.CertManager, + scepConfigService fleet.SCEPConfigService, ) (fleet.Service, error) { authorizer, err := authz.NewAuthorizer() if err != nil { @@ -138,6 +140,7 @@ func NewService( mdmAppleCommander: apple_mdm.NewMDMAppleCommander(mdmStorage, mdmPushService), cronSchedulesService: cronSchedulesService, wstepCertManager: wstepCertManager, + scepConfigService: scepConfigService, } return validationMiddleware{svc, ds, sso}, nil } diff --git a/server/service/testing_utils.go b/server/service/testing_utils.go index 23087e6923..abdf875fb7 100644 --- a/server/service/testing_utils.go +++ b/server/service/testing_utils.go @@ -42,7 +42,6 @@ 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" @@ -69,6 +68,7 @@ func newTestServiceWithConfig(t *testing.T, ds fleet.Datastore, fleetConfig conf depStorage nanodep_storage.AllDEPStorage = &nanodep_mock.Storage{} mailer fleet.MailService = &mockMailService{SendEmailFn: func(e fleet.Email) error { return nil }} c clock.Clock = clock.C + scepConfigService = eeservice.NewSCEPConfigService(logger, nil) mdmStorage fleet.MDMAppleStore mdmPusher nanomdm_push.Pusher @@ -157,6 +157,9 @@ func newTestServiceWithConfig(t *testing.T, ds fleet.Datastore, fleetConfig conf } } } + if len(opts) > 0 && opts[0].SCEPConfigService != nil { + scepConfigService = opts[0].SCEPConfigService + } var wstepManager microsoft_mdm.CertManager if fleetConfig.MDM.WindowsWSTEPIdentityCert != "" && fleetConfig.MDM.WindowsWSTEPIdentityKey != "" { @@ -190,6 +193,7 @@ func newTestServiceWithConfig(t *testing.T, ds fleet.Datastore, fleetConfig conf mdmPusher, cronSchedulesService, wstepManager, + scepConfigService, ) if err != nil { panic(err) @@ -341,6 +345,7 @@ type TestServerOpts struct { EnableSCEPProxy bool WithDEPWebview bool FeatureRoutes []endpoint_utils.HandlerRoutesFunc + SCEPConfigService fleet.SCEPConfigService } func RunServerForTestsWithDS(t *testing.T, ds fleet.Datastore, opts ...*TestServerOpts) (map[string]fleet.User, *httptest.Server) { @@ -399,24 +404,21 @@ func RunServerForTestsWithServiceWithDS(t *testing.T, ctx context.Context, ds fl require.NoError(t, err) } if opts[0].EnableSCEPProxy { + var timeout *time.Duration + if opts[0].SCEPConfigService != nil { + scepConfig, ok := opts[0].SCEPConfigService.(*eeservice.SCEPConfigService) + if ok { + // In tests, we share the same Timeout pointer between SCEPConfigService and SCEPProxy + timeout = scepConfig.Timeout + } + } err := RegisterSCEPProxy( rootMux, ds, logger, + timeout, ) 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 - } } }