From 808d6a000715b73b6b761e1ecc8b3536ef954880 Mon Sep 17 00:00:00 2001 From: Victor Lyuboslavsky Date: Mon, 14 Oct 2024 15:55:52 -0500 Subject: [PATCH] Added activity feed items for NDES SCEP proxy config. (#22902) For #21955 (the story has a video demo of core functionality) Follow up for PR #22542 # Checklist for submitter - [x] Added/updated tests - [x] Manual QA for all new/changed functionality --- docs/Contributing/Audit-logs.md | 18 +++++++++++++++ server/fleet/activities.go | 34 ++++++++++++++++++++++++++++ server/service/appconfig.go | 39 +++++++++++++++++++++++++++----- server/service/appconfig_test.go | 33 +++++++++++++++++++++++++++ 4 files changed, 118 insertions(+), 6 deletions(-) diff --git a/docs/Contributing/Audit-logs.md b/docs/Contributing/Audit-logs.md index 2f12c87ff2..12ee799dd9 100644 --- a/docs/Contributing/Audit-logs.md +++ b/docs/Contributing/Audit-logs.md @@ -1374,6 +1374,24 @@ This activity contains the following fields: } ``` +## added_ndes_scep_proxy + +Generated when NDES SCEP proxy is configured in Fleet. + +This activity does not contain any detail fields. + +## deleted_ndes_scep_proxy + +Generated when NDES SCEP proxy configuration is deleted in Fleet. + +This activity does not contain any detail fields. + +## edited_ndes_scep_proxy + +Generated when NDES SCEP proxy configuration is edited in Fleet. + +This activity does not contain any detail fields. + diff --git a/server/fleet/activities.go b/server/fleet/activities.go index fcf91d4f87..497e439567 100644 --- a/server/fleet/activities.go +++ b/server/fleet/activities.go @@ -108,6 +108,10 @@ var ActivityDetailsList = []ActivityDetails{ ActivityAddedAppStoreApp{}, ActivityDeletedAppStoreApp{}, ActivityInstalledAppStoreApp{}, + + ActivityAddedNDESSCEPProxy{}, + ActivityDeletedNDESSCEPProxy{}, + ActivityEditedNDESSCEPProxy{}, } type ActivityDetails interface { @@ -1851,3 +1855,33 @@ func (a ActivityInstalledAppStoreApp) Documentation() (string, string, string) { "command_uuid": "98765432-1234-1234-1234-1234567890ab" }` } + +type ActivityAddedNDESSCEPProxy struct{} + +func (a ActivityAddedNDESSCEPProxy) ActivityName() string { + return "added_ndes_scep_proxy" +} + +func (a ActivityAddedNDESSCEPProxy) Documentation() (activity string, details string, detailsExample string) { + return "Generated when NDES SCEP proxy is configured in Fleet.", `This activity does not contain any detail fields.`, `` +} + +type ActivityDeletedNDESSCEPProxy struct{} + +func (a ActivityDeletedNDESSCEPProxy) ActivityName() string { + return "deleted_ndes_scep_proxy" +} + +func (a ActivityDeletedNDESSCEPProxy) Documentation() (activity string, details string, detailsExample string) { + return "Generated when NDES SCEP proxy configuration is deleted in Fleet.", `This activity does not contain any detail fields.`, `` +} + +type ActivityEditedNDESSCEPProxy struct{} + +func (a ActivityEditedNDESSCEPProxy) ActivityName() string { + return "edited_ndes_scep_proxy" +} + +func (a ActivityEditedNDESSCEPProxy) Documentation() (activity string, details string, detailsExample string) { + return "Generated when NDES SCEP proxy configuration is edited in Fleet.", `This activity does not contain any detail fields.`, `` +} diff --git a/server/service/appconfig.go b/server/service/appconfig.go index 3f08289f83..c97fbaa7a7 100644 --- a/server/service/appconfig.go +++ b/server/service/appconfig.go @@ -335,6 +335,14 @@ func (svc *Service) ModifyAppConfig(ctx context.Context, p []byte, applyOpts fle return nil, ctxerr.Wrap(ctx, err) } + type ndesStatusType string + const ( + ndesStatusAdded ndesStatusType = "added" + ndesStatusEdited ndesStatusType = "edited" + ndesStatusDeleted ndesStatusType = "deleted" + ) + var ndesStatus ndesStatusType + // Validate NDES SCEP URLs if they changed. Validation is done in both dry run and normal mode. if newAppConfig.Integrations.NDESSCEPProxy.Set && newAppConfig.Integrations.NDESSCEPProxy.Valid && !license.IsPremium() { invalid.Append("integrations.ndes_scep_proxy", ErrMissingLicense.Error()) @@ -347,12 +355,7 @@ func (svc *Service) ModifyAppConfig(ctx context.Context, p []byte, applyOpts fle case !newAppConfig.Integrations.NDESSCEPProxy.Valid: // User is explicitly clearing this setting appConfig.Integrations.NDESSCEPProxy.Valid = false - // Delete stored password - if !applyOpts.DryRun { - if err := svc.ds.HardDeleteMDMConfigAsset(ctx, fleet.MDMAssetNDESPassword); err != nil { - return nil, ctxerr.Wrap(ctx, err, "delete NDES SCEP password") - } - } + ndesStatus = ndesStatusDeleted default: // User is updating the setting appConfig.Integrations.NDESSCEPProxy.Value.URL = fleet.Preprocess(newAppConfig.Integrations.NDESSCEPProxy.Value.URL) @@ -367,15 +370,18 @@ func (svc *Service) ModifyAppConfig(ctx context.Context, p []byte, applyOpts fle validateAdminURL, validateSCEPURL := false, false newSCEPProxy := appConfig.Integrations.NDESSCEPProxy.Value if !oldAppConfig.Integrations.NDESSCEPProxy.Valid { + ndesStatus = ndesStatusAdded validateAdminURL, validateSCEPURL = true, true } else { oldSCEPProxy := oldAppConfig.Integrations.NDESSCEPProxy.Value if newSCEPProxy.URL != oldSCEPProxy.URL { + ndesStatus = ndesStatusEdited validateSCEPURL = true } if newSCEPProxy.AdminURL != oldSCEPProxy.AdminURL || newSCEPProxy.Username != oldSCEPProxy.Username || (newSCEPProxy.Password != "" && newSCEPProxy.Password != fleet.MaskedPassword) { + ndesStatus = ndesStatusEdited validateAdminURL = true } } @@ -587,6 +593,27 @@ func (svc *Service) ModifyAppConfig(ctx context.Context, p []byte, applyOpts fle return nil, err } + switch ndesStatus { + case ndesStatusAdded: + if err = svc.NewActivity(ctx, authz.UserFromContext(ctx), fleet.ActivityAddedNDESSCEPProxy{}); err != nil { + return nil, ctxerr.Wrap(ctx, err, "create activity for added NDES SCEP proxy") + } + case ndesStatusEdited: + if err = svc.NewActivity(ctx, authz.UserFromContext(ctx), fleet.ActivityEditedNDESSCEPProxy{}); err != nil { + return nil, ctxerr.Wrap(ctx, err, "create activity for edited NDES SCEP proxy") + } + case ndesStatusDeleted: + // Delete stored password + if err := svc.ds.HardDeleteMDMConfigAsset(ctx, fleet.MDMAssetNDESPassword); err != nil { + return nil, ctxerr.Wrap(ctx, err, "delete NDES SCEP password") + } + if err = svc.NewActivity(ctx, authz.UserFromContext(ctx), fleet.ActivityDeletedNDESSCEPProxy{}); err != nil { + return nil, ctxerr.Wrap(ctx, err, "create activity for deleted NDES SCEP proxy") + } + default: + // No change, no activity. + } + if oldAppConfig.MDM.MacOSSetup.MacOSSetupAssistant.Value != appConfig.MDM.MacOSSetup.MacOSSetupAssistant.Value && appConfig.MDM.MacOSSetup.MacOSSetupAssistant.Value == "" { // clear macos setup assistant for no team - note that we cannot call diff --git a/server/service/appconfig_test.go b/server/service/appconfig_test.go index 150db25a1b..f50db4cfcf 100644 --- a/server/service/appconfig_test.go +++ b/server/service/appconfig_test.go @@ -1474,6 +1474,11 @@ func TestModifyAppConfigForNDESSCEPProxy(t *testing.T) { fleetConfig := config.TestConfig() svc, ctx = newTestServiceWithConfig(t, ds, fleetConfig, nil, nil, &TestServerOpts{License: &fleet.LicenseInfo{Tier: fleet.TierPremium}}) 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) error { + assert.IsType(t, fleet.ActivityAddedNDESSCEPProxy{}, activity) + return nil + } ac, err := svc.ModifyAppConfig(ctx, []byte(jsonPayload), fleet.ApplySpecOptions{}) require.NoError(t, err) checkSCEPProxy := func() { @@ -1486,6 +1491,10 @@ func TestModifyAppConfigForNDESSCEPProxy(t *testing.T) { checkSCEPProxy() assert.True(t, validateNDESSCEPURLCalled) assert.True(t, validateNDESSCEPAdminURLCalled) + assert.True(t, ds.SaveAppConfigFuncInvoked) + ds.SaveAppConfigFuncInvoked = false + assert.True(t, ds.NewActivityFuncInvoked) + ds.NewActivityFuncInvoked = false // Validation not done if there is no change appConfig = ac @@ -1497,6 +1506,8 @@ func TestModifyAppConfigForNDESSCEPProxy(t *testing.T) { checkSCEPProxy() assert.False(t, validateNDESSCEPURLCalled) assert.False(t, validateNDESSCEPAdminURLCalled) + assert.False(t, ds.NewActivityFuncInvoked) + ds.NewActivityFuncInvoked = false // Validation not done if there is no change, part 2 validateNDESSCEPURLCalled = false @@ -1506,10 +1517,17 @@ func TestModifyAppConfigForNDESSCEPProxy(t *testing.T) { checkSCEPProxy() assert.False(t, validateNDESSCEPURLCalled) assert.False(t, validateNDESSCEPAdminURLCalled) + assert.False(t, ds.NewActivityFuncInvoked) + ds.NewActivityFuncInvoked = false // Validation done for SCEP URL. Password is blank, which is not considered a change. scepURL = "https://new.com/mscep/mscep.dll" jsonPayload = fmt.Sprintf(jsonPayloadBase, scepURL, adminURL, username, "") + ds.NewActivityFunc = func(ctx context.Context, user *fleet.User, activity fleet.ActivityDetails, details []byte, + createdAt time.Time) error { + assert.IsType(t, fleet.ActivityEditedNDESSCEPProxy{}, activity) + return nil + } ac, err = svc.ModifyAppConfig(ctx, []byte(jsonPayload), fleet.ApplySpecOptions{}) require.NoError(t, err) checkSCEPProxy() @@ -1518,6 +1536,8 @@ func TestModifyAppConfigForNDESSCEPProxy(t *testing.T) { appConfig = ac validateNDESSCEPURLCalled = false validateNDESSCEPAdminURLCalled = false + assert.True(t, ds.NewActivityFuncInvoked) + ds.NewActivityFuncInvoked = false // Validation done for SCEP admin URL adminURL = "https://new.com/mscep_admin/" @@ -1527,6 +1547,8 @@ func TestModifyAppConfigForNDESSCEPProxy(t *testing.T) { checkSCEPProxy() assert.False(t, validateNDESSCEPURLCalled) assert.True(t, validateNDESSCEPAdminURLCalled) + assert.True(t, ds.NewActivityFuncInvoked) + ds.NewActivityFuncInvoked = false // Validation fails validateNDESSCEPURLCalled = false @@ -1545,6 +1567,8 @@ func TestModifyAppConfigForNDESSCEPProxy(t *testing.T) { assert.ErrorContains(t, err, "**invalid**") assert.True(t, validateNDESSCEPURLCalled) assert.True(t, validateNDESSCEPAdminURLCalled) + assert.False(t, ds.NewActivityFuncInvoked) + ds.NewActivityFuncInvoked = false // Reset validation validateNDESSCEPURLCalled = false @@ -1577,8 +1601,15 @@ func TestModifyAppConfigForNDESSCEPProxy(t *testing.T) { assert.False(t, validateNDESSCEPURLCalled) assert.False(t, validateNDESSCEPAdminURLCalled) assert.False(t, ds.HardDeleteMDMConfigAssetFuncInvoked, "DB write should not happen in dry run") + assert.False(t, ds.NewActivityFuncInvoked) + ds.NewActivityFuncInvoked = false // Second, real run. + ds.NewActivityFunc = func(ctx context.Context, user *fleet.User, activity fleet.ActivityDetails, details []byte, + createdAt time.Time) error { + assert.IsType(t, fleet.ActivityDeletedNDESSCEPProxy{}, activity) + return nil + } ds.HardDeleteMDMConfigAssetFunc = func(ctx context.Context, assetName fleet.MDMAssetName) error { return nil } @@ -1590,6 +1621,8 @@ func TestModifyAppConfigForNDESSCEPProxy(t *testing.T) { assert.False(t, validateNDESSCEPURLCalled) assert.False(t, validateNDESSCEPAdminURLCalled) assert.True(t, ds.HardDeleteMDMConfigAssetFuncInvoked) + assert.True(t, ds.NewActivityFuncInvoked) + ds.NewActivityFuncInvoked = false // Cannot configure NDES without private key fleetConfig.Server.PrivateKey = ""