Merge pull request #3104 from flipt-io/authz-request-refactor

chore: refactor request models to include scope

internal/server/audit/cloud/cloud_test.go

@@ -27,13 +27,13 @@ func TestSink(t *testing.T) {
 	err := s.SendAudits(context.TODO(), []audit.Event{
 		{
 			Version: "0.1",
-			Type:    flipt.SubjectFlag,
-			Action:  flipt.ActionCreate,
+			Type:    string(flipt.SubjectFlag),
+			Action:  string(flipt.ActionCreate),
 		},
 		{
 			Version: "0.1",
-			Type:    flipt.SubjectConstraint,
-			Action:  flipt.ActionUpdate,
+			Type:    string(flipt.SubjectConstraint),
+			Action:  string(flipt.ActionUpdate),
 		},
 	})
 

internal/server/audit/events.go

@@ -25,9 +25,9 @@ const (
 type Event struct {
 	Version string `json:"version"`
 
-	Type flipt.Subject `json:"type"`
+	Type string `json:"type"`
 
-	Action flipt.Action `json:"-"`
+	Action string `json:"action"`
 
 	Metadata Metadata `json:"metadata"`
 
@@ -36,26 +36,25 @@ type Event struct {
 	Timestamp string `json:"timestamp"`
 }
 
-func (e Event) MarshalJSON() ([]byte, error) {
-
-	type EventAlias Event
-
-	return json.Marshal(&struct {
-		*EventAlias
-		Action string `json:"action"`
-	}{
-		EventAlias: (*EventAlias)(&e),
-		Action:     e.Action.Past(),
-	})
-
-}
-
 // NewEvent is the constructor for an event.
 func NewEvent(r flipt.Request, actor *Actor, payload interface{}) *Event {
+	var action string
+
+	switch r.Action {
+	case flipt.ActionCreate:
+		action = "created"
+	case flipt.ActionUpdate:
+		action = "updated"
+	case flipt.ActionDelete:
+		action = "deleted"
+	default:
+		action = "read"
+	}
+
 	return &Event{
 		Version: eventVersion,
-		Action:  r.Action,
-		Type:    r.Subject,
+		Action:  action,
+		Type:    string(r.Subject),
 		Metadata: Metadata{
 			Actor: actor,
 		},
@@ -79,14 +78,14 @@ func (e Event) DecodeToAttributes() []attribute.KeyValue {
 	if e.Action != "" {
 		akv = append(akv, attribute.KeyValue{
 			Key:   eventActionKey,
-			Value: attribute.StringValue(string(e.Action)),
+			Value: attribute.StringValue(e.Action),
 		})
 	}
 
 	if e.Type != "" {
 		akv = append(akv, attribute.KeyValue{
 			Key:   eventTypeKey,
-			Value: attribute.StringValue(string(e.Type)),
+			Value: attribute.StringValue(e.Type),
 		})
 	}
 
@@ -138,9 +137,9 @@ func decodeToEvent(kvs []attribute.KeyValue) (*Event, error) {
 		case eventVersionKey:
 			e.Version = kv.Value.AsString()
 		case eventActionKey:
-			e.Action = flipt.Action(kv.Value.AsString())
+			e.Action = kv.Value.AsString()
 		case eventTypeKey:
-			e.Type = flipt.Subject(kv.Value.AsString())
+			e.Type = kv.Value.AsString()
 		case eventTimestampKey:
 			e.Timestamp = kv.Value.AsString()
 		case eventMetadataActorKey:

internal/server/audit/logfile/logfile_test.go

@@ -3,6 +3,7 @@ package logfile
 import (
 	"bytes"
 	"context"
+	"encoding/json"
 	"errors"
 	"os"
 	"path/filepath"
@@ -207,15 +208,19 @@ func TestSink_SendAudits(t *testing.T) {
 	require.NoError(t, err)
 	require.NotNil(t, sink)
 
-	assert.NoError(t, sink.SendAudits(context.Background(), []audit.Event{
-		{
-			Version: "1",
-			Type:    flipt.SubjectFlag,
-			Action:  flipt.ActionCreate,
-		},
-	}))
+	e := audit.NewEvent(flipt.NewRequest(flipt.SubjectFlag, flipt.ActionCreate), nil, nil)
+	assert.NoError(t, sink.SendAudits(context.Background(), []audit.Event{*e}))
+
+	assert.NotNil(t, f.Buffer)
+
+	ee := &audit.Event{}
+	err = json.Unmarshal(f.Bytes(), ee)
+	require.NoError(t, err)
+
+	assert.NotEmpty(t, ee.Version)
+	assert.Equal(t, e.Type, ee.Type)
+	assert.Equal(t, e.Action, ee.Action)
+	assert.NotEmpty(t, ee.Timestamp)
 
-	assert.JSONEq(t, `{"version":"1","type":"flag","action":"created","metadata":{},"payload":null,"timestamp":""}
-`, f.Buffer.String())
 	assert.NoError(t, sink.Close())
 }

internal/server/audit/template/executer_test.go

@@ -44,8 +44,8 @@ func TestExecuter_JSON_Failure(t *testing.T) {
 	}
 
 	err = whTemplate.Execute(context.TODO(), audit.Event{
-		Type:   flipt.SubjectFlag,
-		Action: flipt.ActionCreate,
+		Type:   string(flipt.SubjectFlag),
+		Action: string(flipt.ActionCreate),
 	})
 
 	assert.EqualError(t, err, "invalid JSON: this is invalid JSON flag, create")
@@ -67,8 +67,8 @@ func TestExecuter_Execute(t *testing.T) {
 	}
 
 	err = whTemplate.Execute(context.TODO(), audit.Event{
-		Type:   flipt.SubjectFlag,
-		Action: flipt.ActionCreate,
+		Type:   string(flipt.SubjectFlag),
+		Action: string(flipt.ActionCreate),
 	})
 
 	require.NoError(t, err)
@@ -91,8 +91,8 @@ func TestExecuter_Execute_toJson_valid_Json(t *testing.T) {
 	}
 
 	err = whTemplate.Execute(context.TODO(), audit.Event{
-		Type:   flipt.SubjectFlag,
-		Action: flipt.ActionCreate,
+		Type:   string(flipt.SubjectFlag),
+		Action: string(flipt.ActionCreate),
 		Payload: &flipt.CreateFlagRequest{
 			Key:          "foo",
 			Name:         "foo",

internal/server/audit/template/template_test.go

@@ -27,13 +27,13 @@ func TestSink(t *testing.T) {
 	err := s.SendAudits(context.TODO(), []audit.Event{
 		{
 			Version: "0.1",
-			Type:    flipt.SubjectFlag,
-			Action:  flipt.ActionCreate,
+			Type:    string(flipt.SubjectFlag),
+			Action:  string(flipt.ActionCreate),
 		},
 		{
 			Version: "0.1",
-			Type:    flipt.SubjectConstraint,
-			Action:  flipt.ActionUpdate,
+			Type:    string(flipt.SubjectConstraint),
+			Action:  string(flipt.ActionUpdate),
 		},
 	})
 

internal/server/audit/webhook/client.go

@@ -14,9 +14,7 @@ import (
 	"go.uber.org/zap"
 )
 
-const (
-	fliptSignatureHeader = "x-flipt-webhook-signature"
-)
+const fliptSignatureHeader = "x-flipt-webhook-signature"
 
 var _ Client = (*webhookClient)(nil)
 

internal/server/audit/webhook/client_test.go

@@ -40,8 +40,8 @@ func TestWebhookClient(t *testing.T) {
 	}
 
 	err := whclient.SendAudit(context.TODO(), audit.Event{
-		Type:   flipt.SubjectFlag,
-		Action: flipt.ActionCreate,
+		Type:   string(flipt.SubjectFlag),
+		Action: string(flipt.ActionCreate),
 	})
 
 	require.NoError(t, err)

internal/server/audit/webhook/webhook_test.go

@@ -25,13 +25,13 @@ func TestSink(t *testing.T) {
 	err := s.SendAudits(context.TODO(), []audit.Event{
 		{
 			Version: "0.1",
-			Type:    flipt.SubjectFlag,
-			Action:  flipt.ActionCreate,
+			Type:    string(flipt.SubjectFlag),
+			Action:  string(flipt.ActionCreate),
 		},
 		{
 			Version: "0.1",
-			Type:    flipt.SubjectConstraint,
-			Action:  flipt.ActionUpdate,
+			Type:    string(flipt.SubjectConstraint),
+			Action:  string(flipt.ActionUpdate),
 		},
 	})
 

internal/server/authz/engine.go

@@ -27,10 +27,10 @@ default allow = false
 allow {
     input.role != ""
     input.action != ""
-    input.subject != ""
+    input.scope != ""
 
     permissions := get_permissions(input.role)
-    allowed(permissions, input.action, input.subject)
+    allowed(permissions, input.action, input.scope)
 }
 
 get_permissions(role) = result {
@@ -39,28 +39,28 @@ get_permissions(role) = result {
     result = data.roles[idx].rules
 }
 
-allowed(permissions, action, subject) {
+allowed(permissions, action, scope) {
     permissions[action]  # First, ensure the action key exists
-    subject_in_list(permissions[action], subject)  # Check if the subject is in the list
+    scope_in_list(permissions[action], scope)  # Check if the scope is in the list
 }
 
-allowed(permissions, action, subject) {
-    permissions[action]["*"]  # Checks if all subjects are allowed for the action
+allowed(permissions, action, scope) {
+    permissions[action]["*"]  # Checks if all scopes are allowed for the action
 }
 
 # Handles cases where "*" is provided for all actions
-allowed(permissions, action, subject) {
+allowed(permissions, action, scope) {
     permissions["*"] != null  # Check if wildcard for all actions exists
-    subject_in_list(permissions["*"], subject)  # Check if subject is universally allowed or specific to an action
+    scope_in_list(permissions["*"], scope)  # Check if scope is universally allowed or specific to an action
 }
 
 # Helper to handle array membership or wildcard
-subject_in_list(list, subject) {
-    list[_] = subject  # Subject is explicitly listed in the permissions
+scope_in_list(list, scope) {
+    list[_] = scope  # Scope is explicitly listed in the permissions
 }
 
-subject_in_list(list, subject) {
-    list[_] = "*"  # Wildcard entry that permits all subjects
+scope_in_list(list, scope) {
+    list[_] = "*"  # Wildcard entry that permits all scopes
 }
 `
 

internal/server/authz/engine_test.go

@@ -26,7 +26,7 @@ func TestEngine_IsAllowed(t *testing.T) {
 			input: `{
 				"role": "admin",
 				"action": "create",
-				"subject": "flag"
+				"scope": "flag"
 			}`,
 			expected: true,
 		},
@@ -35,7 +35,7 @@ func TestEngine_IsAllowed(t *testing.T) {
 			input: `{
 				"role": "admin",
 				"action": "read",
-				"subject": "flag"
+				"scope": "flag"
 			}`,
 			expected: true,
 		},
@@ -44,7 +44,7 @@ func TestEngine_IsAllowed(t *testing.T) {
 			input: `{
 				"role": "editor",
 				"action": "create",
-				"subject": "flag"
+				"scope": "flag"
 			}`,
 			expected: true,
 		},
@@ -53,7 +53,7 @@ func TestEngine_IsAllowed(t *testing.T) {
 			input: `{
 				"role": "editor",
 				"action": "read",
-				"subject": "flag"
+				"scope": "flag"
 			}`,
 			expected: true,
 		},
@@ -62,7 +62,7 @@ func TestEngine_IsAllowed(t *testing.T) {
 			input: `{
 				"role": "editor",
 				"action": "create",
-				"subject": "namespace"
+				"scope": "namespace"
 			}`,
 			expected: false,
 		},
@@ -71,7 +71,7 @@ func TestEngine_IsAllowed(t *testing.T) {
 			input: `{
 				"role": "viewer",
 				"action": "read",
-				"subject": "flag"
+				"scope": "flag"
 			}`,
 			expected: true,
 		},
@@ -80,7 +80,7 @@ func TestEngine_IsAllowed(t *testing.T) {
 			input: `{
 				"role": "viewer",
 				"action": "create",
-				"subject": "flag"
+				"scope": "flag"
 			}`,
 			expected: false,
 		},

internal/server/authz/middleware/grpc/middleware.go

@@ -80,22 +80,27 @@ func AuthorizationRequiredInterceptor(logger *zap.Logger, policyVerifier authz.V
 		}
 
 		request := requester.Request()
-		action := request.Action
+		action := string(request.Action)
 		if action == "" {
 			logger.Error("unauthorized", zap.String("reason", "request action required"))
 			return ctx, errUnauthorized
 		}
 
-		sub := request.Subject
-		if sub == "" {
-			logger.Error("unauthorized", zap.String("reason", "request subject required"))
+		scope := string(request.Scope)
+		if scope == "" {
+			// fallback to subject if scope is not provided
+			scope = string(request.Subject)
+		}
+
+		if scope == "" {
+			logger.Error("unauthorized", zap.String("reason", "request scope or subject required"))
 			return ctx, errUnauthorized
 		}
 
 		allowed, err := policyVerifier.IsAllowed(ctx, map[string]interface{}{
-			"role":    role,
-			"action":  action,
-			"subject": sub,
+			"role":   role,
+			"action": action,
+			"scope":  scope,
 		})
 
 		if err != nil {