feat: Audit failed authz attempts (#3155)

* feat: authz audit events * chore: continue with audit events for unauthorized requests Signed-off-by: Mark Phelps <[email protected]> * chore: fix unit test Signed-off-by: Mark Phelps <[email protected]> * chore: fix audit event middleware to capture failed authz requests Signed-off-by: Mark Phelps <[email protected]> * chore: proto regen Signed-off-by: Mark Phelps <[email protected]> * chore: bump audit event version Signed-off-by: Mark Phelps <[email protected]> * chore: fix tests Signed-off-by: Mark Phelps <[email protected]> * chore: fix marshal log test Signed-off-by: Mark Phelps <[email protected]> --------- Signed-off-by: Mark Phelps <[email protected]>
flipt-io · Jun 9, 2024 · 113c2cb · 113c2cb
1 parent 997e1d0
commit 113c2cb
Show file tree

Hide file tree

Showing 18 changed files with 291 additions and 160 deletions.
diff --git a/.gitignore b/.gitignore
@@ -55,3 +55,4 @@ devenv.local.nix
 .pre-commit-config.yaml
 
 build/mage_output_file.go
+*.rego
diff --git a/errors/errors.go b/errors/errors.go
@@ -90,6 +90,14 @@ func (e ErrUnauthenticated) Error() string {
 	return string(e)
 }
 
+type ErrUnauthorized string
+
+var ErrUnauthorizedf = NewErrorf[ErrUnauthorized]
+
+func (e ErrUnauthorized) Error() string {
+	return string(e)
+}
+
 // StringError is any error that also happens to have an underlying type of string.
 type StringError interface {
 	error

diff --git a/internal/cmd/grpc.go b/internal/cmd/grpc.go
@@ -267,7 +267,7 @@ func NewGRPCServer(
 	skipAuthnIfExcluded(evalsrv, cfg.Authentication.Exclude.Evaluation)
 	skipAuthnIfExcluded(evaldatasrv, cfg.Authentication.Exclude.Evaluation)
 
-	var checker *audit.Checker
+	var checker audit.EventPairChecker = &audit.NoOpChecker{}
 
 	// We have to check if audit logging is enabled here for informing the authentication service that
 	// the user would like to receive token:deleted events.
@@ -345,50 +345,6 @@ func NewGRPCServer(
 		)...,
 	)
 
-	// cache must come after auth interceptors
-	if cfg.Cache.Enabled && cacher != nil {
-		interceptors = append(interceptors, middlewaregrpc.CacheUnaryInterceptor(cacher, logger))
-	}
-
-	if cfg.Authorization.Required {
-		authzOpts := []containers.Option[authzmiddlewaregrpc.InterceptorOptions]{
-			authzmiddlewaregrpc.WithServerSkipsAuthorization(healthsrv),
-		}
-
-		engineOpts := []containers.Option[authz.Engine]{
-			authz.WithPollDuration(cfg.Authorization.Policy.PollInterval),
-		}
-
-		if cfg.Authorization.Data != nil {
-			switch cfg.Authorization.Data.Backend {
-			case config.AuthorizationBackendLocal:
-				engineOpts = append(engineOpts, authz.WithDataSource(
-					filesystem.DataSourceFromPath(cfg.Authorization.Data.Local.Path),
-					cfg.Authorization.Data.PollInterval,
-				))
-			default:
-				return nil, fmt.Errorf("unexpected authz data backend type: %q", cfg.Authorization.Data.Backend)
-			}
-		}
-
-		var source authz.PolicySource
-		switch cfg.Authorization.Policy.Backend {
-		case config.AuthorizationBackendLocal:
-			source = filesystem.PolicySourceFromPath(cfg.Authorization.Policy.Local.Path)
-		default:
-			return nil, fmt.Errorf("unexpected authz policy backend type: %q", cfg.Authorization.Policy.Backend)
-		}
-
-		policyEngine, err := authz.NewEngine(ctx, logger, source, engineOpts...)
-		if err != nil {
-			return nil, fmt.Errorf("creating authorization policy engine: %w", err)
-		}
-
-		interceptors = append(interceptors, authzmiddlewaregrpc.AuthorizationRequiredInterceptor(logger, policyEngine, authzOpts...))
-
-		logger.Info("authorization middleware enabled")
-	}
-
 	// audit sinks configuration
 	sinks := make([]audit.Sink, 0)
 
@@ -488,6 +444,50 @@ func NewGRPCServer(
 	}
 	otel.SetTextMapPropagator(textMapPropagator)
 
+	if cfg.Authorization.Required {
+		authzOpts := []containers.Option[authzmiddlewaregrpc.InterceptorOptions]{
+			authzmiddlewaregrpc.WithServerSkipsAuthorization(healthsrv),
+		}
+
+		engineOpts := []containers.Option[authz.Engine]{
+			authz.WithPollDuration(cfg.Authorization.Policy.PollInterval),
+		}
+
+		if cfg.Authorization.Data != nil {
+			switch cfg.Authorization.Data.Backend {
+			case config.AuthorizationBackendLocal:
+				engineOpts = append(engineOpts, authz.WithDataSource(
+					filesystem.DataSourceFromPath(cfg.Authorization.Data.Local.Path),
+					cfg.Authorization.Data.PollInterval,
+				))
+			default:
+				return nil, fmt.Errorf("unexpected authz data backend type: %q", cfg.Authorization.Data.Backend)
+			}
+		}
+
+		var source authz.PolicySource
+		switch cfg.Authorization.Policy.Backend {
+		case config.AuthorizationBackendLocal:
+			source = filesystem.PolicySourceFromPath(cfg.Authorization.Policy.Local.Path)
+		default:
+			return nil, fmt.Errorf("unexpected authz policy backend type: %q", cfg.Authorization.Policy.Backend)
+		}
+
+		policyEngine, err := authz.NewEngine(ctx, logger, source, engineOpts...)
+		if err != nil {
+			return nil, fmt.Errorf("creating authorization policy engine: %w", err)
+		}
+
+		interceptors = append(interceptors, authzmiddlewaregrpc.AuthorizationRequiredInterceptor(logger, policyEngine, authzOpts...))
+
+		logger.Info("authorization middleware enabled")
+	}
+
+	// cache must come after authn and authz interceptors
+	if cfg.Cache.Enabled && cacher != nil {
+		interceptors = append(interceptors, middlewaregrpc.CacheUnaryInterceptor(cacher, logger))
+	}
+
 	grpcOpts := []grpc.ServerOption{
 		grpc.ChainUnaryInterceptor(interceptors...),
 		grpc.KeepaliveParams(keepalive.ServerParameters{

diff --git a/internal/server/analytics/sink.go b/internal/server/analytics/sink.go
@@ -42,10 +42,12 @@ func NewAnalyticsSinkSpanExporter(logger *zap.Logger, analyticsStoreMutator Anal
 	}
 }
 
+const evaluationResponseKey = "flipt.evaluation.response"
+
 // transformSpanEventToEvaluationResponses is a convenience function to transform a span event into an []*EvaluationResponse.
 func transformSpanEventToEvaluationResponses(event sdktrace.Event) ([]*EvaluationResponse, error) {
 	for _, attr := range event.Attributes {
-		if string(attr.Key) == "flipt.evaluation.response" {
+		if string(attr.Key) == evaluationResponseKey {
 			evaluationResponseBytes := []byte(attr.Value.AsString())
 			var evaluationResponse []*EvaluationResponse
 

diff --git a/internal/server/audit/checker.go b/internal/server/audit/checker.go
@@ -6,6 +6,12 @@ import (
 	"strings"
 )
 
+// EventPairChecker is the contract for checking if an event pair exists and if it should be emitted to configured sinks.
+type EventPairChecker interface {
+	Check(eventPair string) bool
+	Events() []string
+}
+
 // Checker holds a map that maps event pairs to a dummy struct. It is basically
 // used as a set to check for existence.
 type Checker struct {
@@ -93,3 +99,13 @@ func (c *Checker) Events() []string {
 
 	return events
 }
+
+type NoOpChecker struct{}
+
+func (n *NoOpChecker) Check(eventPair string) bool {
+	return false
+}
+
+func (n *NoOpChecker) Events() []string {
+	return []string{}
+}
diff --git a/internal/server/audit/events.go b/internal/server/audit/events.go
@@ -13,16 +13,17 @@ import (
 )
 
 const (
-	eventVersion          = "0.1"
+	eventVersion          = "0.2"
 	eventVersionKey       = "flipt.event.version"
 	eventActionKey        = "flipt.event.action"
 	eventTypeKey          = "flipt.event.type"
 	eventMetadataActorKey = "flipt.event.metadata.actor"
 	eventPayloadKey       = "flipt.event.payload"
 	eventTimestampKey     = "flipt.event.timestamp"
+	eventStatusKey        = "flipt.event.status"
 )
 
-// Event holds information that represents an action that has occurred in the system.
+// Event holds information that represents an action that was attempted in the system.
 type Event struct {
 	Version string `json:"version"`
 
@@ -35,6 +36,8 @@ type Event struct {
 	Payload interface{} `json:"payload"`
 
 	Timestamp string `json:"timestamp"`
+
+	Status string `json:"status"`
 }
 
 // NewEvent is the constructor for an event.
@@ -61,6 +64,7 @@ func NewEvent(r flipt.Request, actor *Actor, payload interface{}) *Event {
 		Version: eventVersion,
 		Action:  action,
 		Type:    typ,
+		Status:  string(r.Status),
 		Metadata: Metadata{
 			Actor: actor,
 		},
@@ -102,6 +106,13 @@ func (e Event) DecodeToAttributes() []attribute.KeyValue {
 		})
 	}
 
+	if e.Status != "" {
+		akv = append(akv, attribute.KeyValue{
+			Key:   eventStatusKey,
+			Value: attribute.StringValue(e.Status),
+		})
+	}
+
 	b, err := json.Marshal(e.Metadata.Actor)
 	if err == nil {
 		akv = append(akv, attribute.KeyValue{
@@ -129,13 +140,14 @@ func (e *Event) AddToSpan(ctx context.Context) {
 }
 
 func (e *Event) Valid() bool {
-	return e.Version != "" && e.Action != "" && e.Type != "" && e.Timestamp != "" && e.Payload != nil
+	return e.Version != "" && e.Action != "" && e.Type != "" && e.Timestamp != ""
 }
 
 func (e Event) MarshalLogObject(enc zapcore.ObjectEncoder) error {
 	enc.AddString("version", e.Version)
 	enc.AddString("type", e.Type)
 	enc.AddString("action", e.Action)
+	enc.AddString("status", e.Status)
 	if err := enc.AddReflected("metadata", e.Metadata); err != nil {
 		return err
 	}
@@ -162,6 +174,8 @@ func decodeToEvent(kvs []attribute.KeyValue) (*Event, error) {
 			e.Type = kv.Value.AsString()
 		case eventTimestampKey:
 			e.Timestamp = kv.Value.AsString()
+		case eventStatusKey:
+			e.Status = kv.Value.AsString()
 		case eventMetadataActorKey:
 			var actor Actor
 			if err := json.Unmarshal([]byte(kv.Value.AsString()), &actor); err != nil {

diff --git a/internal/server/audit/events_test.go b/internal/server/audit/events_test.go
@@ -9,23 +9,28 @@ import (
 )
 
 func TestMarshalLogObject(t *testing.T) {
-	actor := Actor{Authentication: "github"}
-	e := Event{
-		Version:  "0.1",
-		Type:     "sometype",
-		Action:   "modified",
-		Metadata: Metadata{Actor: &actor},
-		Payload:  "custom payload",
-	}
-	enc := zapcore.NewMapObjectEncoder()
-	err := e.MarshalLogObject(enc)
+	var (
+		actor = Actor{Authentication: "github"}
+		e     = Event{
+			Version:  "0.2",
+			Type:     "sometype",
+			Action:   "modified",
+			Metadata: Metadata{Actor: &actor},
+			Payload:  "custom payload",
+			Status:   "success",
+		}
+		enc = zapcore.NewMapObjectEncoder()
+		err = e.MarshalLogObject(enc)
+	)
+
 	require.NoError(t, err)
 	assert.Equal(t, map[string]any{
-		"version":   "0.1",
+		"version":   "0.2",
 		"action":    "modified",
 		"type":      "sometype",
 		"metadata":  Metadata{Actor: &actor},
 		"payload":   "custom payload",
 		"timestamp": "",
+		"status":    "success",
 	}, enc.Fields)
 }
diff --git a/internal/server/audit/log/log_test.go b/internal/server/audit/log/log_test.go
@@ -33,14 +33,16 @@ func TestSink(t *testing.T) {
 
 	err = s.SendAudits(context.TODO(), []audit.Event{
 		{
-			Version: "0.1",
+			Version: "0.2",
 			Type:    string(flipt.SubjectFlag),
 			Action:  string(flipt.ActionCreate),
+			Status:  string(flipt.StatusSuccess),
 		},
 		{
-			Version: "0.1",
+			Version: "0.2",
 			Type:    string(flipt.SubjectConstraint),
 			Action:  string(flipt.ActionUpdate),
+			Status:  string(flipt.StatusSuccess),
 		},
 	})
 
@@ -58,7 +60,7 @@ func TestSink(t *testing.T) {
 	assert.NotEmpty(t, lines)
 	assert.NotEmpty(t, lines[0])
 
-	assert.JSONEq(t, `{"version": "0.1", "type": "flag", "action": "create", "metadata": {}, "payload": null, "timestamp": ""}`, lines[0])
+	assert.JSONEq(t, `{"version": "0.2", "type": "flag", "action": "create", "metadata": {}, "payload": null, "timestamp": "", "status": "success"}`, lines[0])
 }
 
 func TestSink_DirNotExists(t *testing.T) {
@@ -87,14 +89,16 @@ func TestSink_DirNotExists(t *testing.T) {
 
 			err = s.SendAudits(context.TODO(), []audit.Event{
 				{
-					Version: "0.1",
+					Version: "0.2",
 					Type:    string(flipt.SubjectFlag),
 					Action:  string(flipt.ActionCreate),
+					Status:  string(flipt.StatusSuccess),
 				},
 				{
-					Version: "0.1",
+					Version: "0.2",
 					Type:    string(flipt.SubjectConstraint),
 					Action:  string(flipt.ActionUpdate),
+					Status:  string(flipt.StatusSuccess),
 				},
 			})
 
@@ -113,7 +117,7 @@ func TestSink_DirNotExists(t *testing.T) {
 			assert.NotEmpty(t, lines)
 			assert.NotEmpty(t, lines[0])
 
-			assert.JSONEq(t, `{"version": "0.1", "type": "flag", "action": "create", "metadata": {}, "payload": null, "timestamp": ""}`, lines[0])
+			assert.JSONEq(t, `{"version": "0.2", "type": "flag", "action": "create", "metadata": {}, "payload": null, "timestamp": "", "status": "success"}`, lines[0])
 		})
 	}
 }