ROX-10845: Move notifier scrubbing to datastore (#1803)

central/debug/service/service.go

@@ -44,7 +44,6 @@ import (
 	"github.com/stackrox/rox/pkg/prometheusutil"
 	"github.com/stackrox/rox/pkg/sac"
 	"github.com/stackrox/rox/pkg/sac/observe"
-	"github.com/stackrox/rox/pkg/secrets"
 	"github.com/stackrox/rox/pkg/telemetry/data"
 	"github.com/stackrox/rox/pkg/version"
 	"google.golang.org/grpc"
@@ -437,15 +436,7 @@ func (s *serviceImpl) getNotifiers(_ context.Context) (interface{}, error) {
 			sac.AccessModeScopeKeys(storage.Access_READ_ACCESS),
 			sac.ResourceScopeKeys(resources.Notifier)))
 
-	notifiers, err := s.notifierDataStore.GetNotifiers(accessNotifierCtx)
-	if err != nil {
-		return nil, err
-	}
-	for _, n := range notifiers {
-		secrets.ScrubSecretsFromStructWithReplacement(n, secrets.ScrubReplacementStr)
-	}
-
-	return notifiers, nil
+	return s.notifierDataStore.GetScrubbedNotifiers(accessNotifierCtx)
 }
 
 func (s *serviceImpl) getConfig(_ context.Context) (interface{}, error) {

central/debug/service/service_test.go

@@ -126,24 +126,10 @@ func (s *DebugServiceTestSuite) TestGetRoles() {
 }
 
 func (s *DebugServiceTestSuite) TestGetNotifiers() {
-	s.notifiersMock.EXPECT().GetNotifiers(gomock.Any()).Return(nil, errors.New("Test"))
+	s.notifiersMock.EXPECT().GetScrubbedNotifiers(gomock.Any()).Return(nil, errors.New("Test"))
 	_, err := s.service.getNotifiers(s.noneCtx)
 	s.Error(err, "expected error propagation")
 
-	notifiers := []*storage.Notifier{
-		{
-			Name: "test",
-			Config: &storage.Notifier_Pagerduty{
-				Pagerduty: &storage.PagerDuty{
-					ApiKey: "test",
-				},
-			},
-		},
-	}
-	s.notifiersMock.EXPECT().GetNotifiers(gomock.Any()).Return(notifiers, nil)
-	actualNotifiers, err := s.service.getNotifiers(s.noneCtx)
-
-	// Set new value in order to avoid comparing with referenced objects.
 	expectedNotifiers := []*storage.Notifier{
 		{
 			Name: "test",
@@ -154,6 +140,8 @@ func (s *DebugServiceTestSuite) TestGetNotifiers() {
 			},
 		},
 	}
+	s.notifiersMock.EXPECT().GetScrubbedNotifiers(gomock.Any()).Return(expectedNotifiers, nil)
+	actualNotifiers, err := s.service.getNotifiers(s.noneCtx)
 
 	s.NoError(err)
 	s.EqualValues(expectedNotifiers, actualNotifiers)

central/graphql/resolvers/notifiers.go

@@ -26,7 +26,7 @@ func (resolver *Resolver) Notifiers(ctx context.Context) ([]*notifierResolver, e
 		return nil, err
 	}
 	return resolver.wrapNotifiers(
-		resolver.NotifierStore.GetNotifiers(ctx))
+		resolver.NotifierStore.GetScrubbedNotifiers(ctx))
 }
 
 // Notifier gets a single notifier by ID
@@ -36,5 +36,5 @@ func (resolver *Resolver) Notifier(ctx context.Context, args struct{ graphql.ID
 		return nil, err
 	}
 	return resolver.wrapNotifier(
-		resolver.NotifierStore.GetNotifier(ctx, string(args.ID)))
+		resolver.NotifierStore.GetScrubbedNotifier(ctx, string(args.ID)))
 }

central/notifier/datastore/datastore.go

@@ -11,7 +11,9 @@ import (
 //go:generate mockgen-wrapper
 type DataStore interface {
 	GetNotifier(ctx context.Context, id string) (*storage.Notifier, bool, error)
+	GetScrubbedNotifier(ctx context.Context, id string) (*storage.Notifier, bool, error)
 	GetNotifiers(ctx context.Context) ([]*storage.Notifier, error)
+	GetScrubbedNotifiers(ctx context.Context) ([]*storage.Notifier, error)
 	AddNotifier(ctx context.Context, notifier *storage.Notifier) (string, error)
 	UpdateNotifier(ctx context.Context, notifier *storage.Notifier) error
 	RemoveNotifier(ctx context.Context, id string) error

central/notifier/datastore/datastore_impl.go

@@ -8,6 +8,7 @@ import (
 	"github.com/stackrox/rox/generated/storage"
 	"github.com/stackrox/rox/pkg/errox"
 	"github.com/stackrox/rox/pkg/sac"
+	"github.com/stackrox/rox/pkg/secrets"
 	"github.com/stackrox/rox/pkg/sync"
 	"github.com/stackrox/rox/pkg/uuid"
 )
@@ -31,6 +32,17 @@ func (b *datastoreImpl) GetNotifier(ctx context.Context, id string) (*storage.No
 	return b.storage.Get(ctx, id)
 }
 
+func (b *datastoreImpl) GetScrubbedNotifier(ctx context.Context, id string) (*storage.Notifier, bool, error) {
+	notifier, exists, err := b.GetNotifier(ctx, id)
+	if err != nil || !exists {
+		return notifier, exists, err
+	}
+
+	secrets.ScrubSecretsFromStructWithReplacement(notifier, secrets.ScrubReplacementStr)
+
+	return notifier, exists, err
+}
+
 func (b *datastoreImpl) GetNotifiers(ctx context.Context) ([]*storage.Notifier, error) {
 	if ok, err := notifierSAC.ReadAllowed(ctx); err != nil {
 		return nil, err
@@ -41,6 +53,19 @@ func (b *datastoreImpl) GetNotifiers(ctx context.Context) ([]*storage.Notifier,
 	return b.storage.GetAll(ctx)
 }
 
+func (b *datastoreImpl) GetScrubbedNotifiers(ctx context.Context) ([]*storage.Notifier, error) {
+	notifiers, err := b.GetNotifiers(ctx)
+	if err != nil {
+		return nil, err
+	}
+
+	for _, notifier := range notifiers {
+		secrets.ScrubSecretsFromStructWithReplacement(notifier, secrets.ScrubReplacementStr)
+	}
+
+	return notifiers, nil
+}
+
 func (b *datastoreImpl) AddNotifier(ctx context.Context, notifier *storage.Notifier) (string, error) {
 	if ok, err := notifierSAC.WriteAllowed(ctx); err != nil {
 		return "", err

central/notifier/datastore/datastsore_test.go

@@ -91,6 +91,23 @@ func (s *notifierDataStoreTestSuite) TestAllowsGet() {
 	s.True(exists, "expected exists to be set to false")
 }
 
+func (s *notifierDataStoreTestSuite) TestGetScrubbedNotifier() {
+	testNotifier := &storage.Notifier{
+		Config: &storage.Notifier_Generic{
+			Generic: &storage.Generic{
+				Password: "test",
+			},
+		},
+	}
+
+	s.storage.EXPECT().Get(gomock.Any(), gomock.Any()).Return(testNotifier, true, nil).Times(1)
+
+	scrubbedNotifier, exists, err := s.dataStore.GetScrubbedNotifier(s.hasReadCtx, "test")
+	s.NoError(err, "expected no error trying to read with permissions")
+	s.True(exists, "expected exists to be set to false")
+	s.Equal("******", scrubbedNotifier.Config.(*storage.Notifier_Generic).Generic.Password)
+}
+
 func (s *notifierDataStoreTestSuite) TestEnforcesGetMany() {
 	s.storage.EXPECT().GetAll(gomock.Any()).Times(0)
 
@@ -117,6 +134,23 @@ func (s *notifierDataStoreTestSuite) TestAllowsGetMany() {
 	s.NoError(err, "expected no error trying to read with Integration permissions")
 }
 
+func (s *notifierDataStoreTestSuite) TestGetScrubbedNotifiers() {
+	testNotifier := &storage.Notifier{
+		Config: &storage.Notifier_Generic{
+			Generic: &storage.Generic{
+				Password: "test",
+			},
+		},
+	}
+
+	s.storage.EXPECT().GetAll(gomock.Any()).Return([]*storage.Notifier{testNotifier}, nil).Times(1)
+
+	scrubbedNotifiers, err := s.dataStore.GetScrubbedNotifiers(s.hasReadCtx)
+	s.NoError(err, "expected no error trying to read with permissions")
+	s.Equal(1, len(scrubbedNotifiers), "expected one notifier in the list")
+	s.Equal("******", scrubbedNotifiers[0].Config.(*storage.Notifier_Generic).Generic.Password)
+}
+
 func (s *notifierDataStoreTestSuite) TestEnforcesAdd() {
 	s.storage.EXPECT().GetAll(gomock.Any()).Times(0)
 

central/notifier/service/service_impl.go

@@ -77,27 +77,25 @@ func (s *serviceImpl) GetNotifier(ctx context.Context, request *v1.ResourceByID)
 	if request.GetId() == "" {
 		return nil, errors.Wrap(errox.InvalidArgs, "notifier id must be provided")
 	}
-	notifier, exists, err := s.storage.GetNotifier(ctx, request.GetId())
+	notifier, exists, err := s.storage.GetScrubbedNotifier(ctx, request.GetId())
 	if err != nil {
 		return nil, err
 	}
 	if !exists {
 		return nil, errors.Wrapf(errox.NotFound, "notifier %v not found", request.GetId())
 	}
-	secrets.ScrubSecretsFromStructWithReplacement(notifier, secrets.ScrubReplacementStr)
+
 	return notifier, nil
 }
 
 // GetNotifiers retrieves all notifiers that match the request filters
 func (s *serviceImpl) GetNotifiers(ctx context.Context, _ *v1.GetNotifiersRequest) (*v1.GetNotifiersResponse, error) {
-	notifiers, err := s.storage.GetNotifiers(ctx)
+	scrubbedNotifiers, err := s.storage.GetScrubbedNotifiers(ctx)
 	if err != nil {
 		return nil, err
 	}
-	for _, n := range notifiers {
-		secrets.ScrubSecretsFromStructWithReplacement(n, secrets.ScrubReplacementStr)
-	}
-	return &v1.GetNotifiersResponse{Notifiers: notifiers}, nil
+
+	return &v1.GetNotifiersResponse{Notifiers: scrubbedNotifiers}, nil
 }
 
 func validateNotifier(notifier *storage.Notifier) error {