fix: check authz for evaluation endpoints (#3438)

* fix: disable authz for all evaluation endpoints

Signed-off-by: Mark Phelps <[email protected]>

* chore: refactor authz test a bit

Signed-off-by: Mark Phelps <[email protected]>

* chore: add missing test

Signed-off-by: Mark Phelps <[email protected]>

* fix(rpc/flipt): implement Requester for evaluation data snapshot (#3439)

* chore: fix lint error

Signed-off-by: Mark Phelps <[email protected]>

* chore: try to fix snapshot IT

Signed-off-by: Mark Phelps <[email protected]>

---------

Signed-off-by: Mark Phelps <[email protected]>
Co-authored-by: George <[email protected]>
Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>

Author: 3 people

build/testing/integration.go

@@ -58,8 +58,8 @@ var (
 		"api/cockroach":     withCockroach(api),
 		"api/cache":         cache,
 		"api/cachetls":      cacheWithTLS,
-		"api/snapshot":      snapshot,
-		"api/ofrep":         ofrep,
+		"api/snapshot":      withAuthz(snapshot),
+		"api/ofrep":         withAuthz(ofrep),
 		"fs/git":            git,
 		"fs/local":          local,
 		"fs/s3":             s3,
@@ -671,18 +671,35 @@ func authn(ctx context.Context, _ *dagger.Client, base, flipt *dagger.Container,
 	return suite(ctx, "authn", base, fliptToTest, conf)
 }
 
-func authz(ctx context.Context, _ *dagger.Client, base, flipt *dagger.Container, conf testConfig) func() error {
-	var (
-		policyPath = "/etc/flipt/authz/policy.rego"
-		policyData = "/etc/flipt/authz/data.json"
-	)
+func authz(ctx context.Context, client *dagger.Client, base, flipt *dagger.Container, conf testConfig) func() error {
+	return withAuthz(func(ctx context.Context, client *dagger.Client, base, flipt *dagger.Container, conf testConfig) func() error {
 
-	// create unique instance for test case
-	fliptToTest := flipt.
-		WithEnvVariable("FLIPT_AUTHORIZATION_REQUIRED", "true").
-		WithEnvVariable("FLIPT_AUTHORIZATION_BACKEND", "local").
-		WithEnvVariable("FLIPT_AUTHORIZATION_LOCAL_POLICY_PATH", policyPath).
-		WithNewFile(policyPath, `package flipt.authz.v1
+		// create unique instance for test case
+		fliptToTest := flipt.
+			WithEnvVariable("UNIQUE", uuid.New().String()).
+			WithExec(nil)
+
+		// import state into instance before running test
+		if err := importInto(ctx, base, flipt, fliptToTest, "--address", conf.address, "--token", bootstrapToken); err != nil {
+			return func() error { return err }
+		}
+
+		return suite(ctx, "authz", base, fliptToTest, conf)
+	})(ctx, client, base, flipt, conf)
+}
+
+func withAuthz(fn testCaseFn) testCaseFn {
+	return func(ctx context.Context, client *dagger.Client, base, flipt *dagger.Container, conf testConfig) func() error {
+		var (
+			policyPath = "/etc/flipt/authz/policy.rego"
+			policyData = "/etc/flipt/authz/data.json"
+		)
+
+		return fn(ctx, client, base, flipt.
+			WithEnvVariable("FLIPT_AUTHORIZATION_REQUIRED", "true").
+			WithEnvVariable("FLIPT_AUTHORIZATION_BACKEND", "local").
+			WithEnvVariable("FLIPT_AUTHORIZATION_LOCAL_POLICY_PATH", policyPath).
+			WithNewFile(policyPath, `package flipt.authz.v1
 
 import data
 import rego.v1
@@ -736,8 +753,8 @@ permit_slice(allowed, _) if {
 permit_slice(allowed, requested) if {
 	allowed[_] = requested
 }`).
-		WithEnvVariable("FLIPT_AUTHORIZATION_LOCAL_DATA_PATH", policyData).
-		WithNewFile(policyData, `{
+			WithEnvVariable("FLIPT_AUTHORIZATION_LOCAL_DATA_PATH", policyData).
+			WithNewFile(policyData, `{
     "version": "0.1.0",
     "roles": [
         {
@@ -822,18 +839,11 @@ permit_slice(allowed, requested) if {
             ]
         }
     ]
-}`).
-		WithEnvVariable("UNIQUE", uuid.New().String()).
-		WithExec(nil)
-
-	// import state into instance before running test
-	if err := importInto(ctx, base, flipt, fliptToTest, "--address", conf.address, "--token", bootstrapToken); err != nil {
-		return func() error { return err }
+}`),
+			conf,
+		)
 	}
-
-	return suite(ctx, "authz", base, fliptToTest, conf)
 }
-
 func withWebhook(fn testCaseFn) testCaseFn {
 	return func(ctx context.Context, client *dagger.Client, base, flipt *dagger.Container, conf testConfig) func() error {
 		owntracks := client.Container().From("frxyt/gohrec").WithExposedPort(8080).AsService()

build/testing/integration/authz/auth.go

@@ -11,6 +11,7 @@ import (
 	"go.flipt.io/build/testing/integration"
 	"go.flipt.io/flipt/rpc/flipt"
 	"go.flipt.io/flipt/rpc/flipt/auth"
+	"go.flipt.io/flipt/rpc/flipt/evaluation"
 	sdk "go.flipt.io/flipt/sdk/go"
 	"google.golang.org/grpc/codes"
 	"google.golang.org/grpc/status"
@@ -19,6 +20,30 @@ import (
 func Common(t *testing.T, opts integration.TestOpts) {
 	client := opts.TokenClient(t)
 
+	t.Run("Evaluation", func(t *testing.T) {
+		ctx := context.Background()
+
+		t.Run("Boolean", func(t *testing.T) {
+			_, err := client.Evaluation().Boolean(ctx, &evaluation.EvaluationRequest{
+				FlagKey: "flag_boolean",
+				Context: map[string]string{
+					"in_segment": "segment_001",
+				},
+			})
+			require.NoError(t, err)
+		})
+
+		t.Run("Variant", func(t *testing.T) {
+			_, err := client.Evaluation().Variant(ctx, &evaluation.EvaluationRequest{
+				FlagKey: "flag_001",
+				Context: map[string]string{
+					"in_segment": "segment_001",
+				},
+			})
+			require.NoError(t, err)
+		})
+	})
+
 	t.Run("Authentication Methods", func(t *testing.T) {
 		ctx := context.Background()
 
@@ -124,19 +149,6 @@ func Common(t *testing.T, opts integration.TestOpts) {
 	})
 }
 
-func listFlagIsAllowed(t *testing.T, ctx context.Context, client sdk.SDK, namespace string) {
-	t.Helper()
-
-	t.Run(fmt.Sprintf("ListFlags(namespace: %q)", namespace), func(t *testing.T) {
-		// construct a new client using the previously obtained client token
-		resp, err := client.Flipt().ListFlags(ctx, &flipt.ListFlagRequest{
-			NamespaceKey: namespace,
-		})
-		require.NoError(t, err)
-		require.NotEmpty(t, resp.Flags)
-	})
-}
-
 func canReadAllIn(t *testing.T, ctx context.Context, client sdk.SDK, namespace string) {
 	t.Run("CanReadAll", func(t *testing.T) {
 		clientCallSet{

build/testing/integration/snapshot/snapshot_test.go

@@ -15,7 +15,7 @@ import (
 func TestSnapshot(t *testing.T) {
 	integration.Harness(t, func(t *testing.T, opts integration.TestOpts) {
 		var (
-			httpClient = opts.HTTPClient(t)
+			httpClient = opts.HTTPClient(t, integration.WithRole("viewer")) // TODO: test other roles/namespace combinations
 			protocol   = opts.Protocol()
 		)
 

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

@@ -90,21 +90,21 @@ func AuthorizationRequiredInterceptor(logger *zap.Logger, policyVerifier authz.V
 			return ctx, errUnauthorized
 		}
 
-		request := requester.Request()
-
-		allowed, err := policyVerifier.IsAllowed(ctx, map[string]interface{}{
-			"request":        request,
-			"authentication": auth,
-		})
-
-		if err != nil {
-			logger.Error("unauthorized", zap.Error(err))
-			return ctx, errUnauthorized
-		}
-
-		if !allowed {
-			logger.Error("unauthorized", zap.String("reason", "permission denied"))
-			return ctx, errUnauthorized
+		for _, request := range requester.Request() {
+			allowed, err := policyVerifier.IsAllowed(ctx, map[string]interface{}{
+				"request":        request,
+				"authentication": auth,
+			})
+
+			if err != nil {
+				logger.Error("unauthorized", zap.Error(err))
+				return ctx, errUnauthorized
+			}
+
+			if !allowed {
+				logger.Error("unauthorized", zap.String("reason", "permission denied"))
+				return ctx, errUnauthorized
+			}
 		}
 
 		return handler(ctx, req)

internal/server/evaluation/server_test.go

@@ -11,3 +11,8 @@ func Test_Server_AllowsNamespaceScopedAuthentication(t *testing.T) {
 	server := &Server{}
 	assert.True(t, server.AllowsNamespaceScopedAuthentication(context.Background()))
 }
+
+func Test_Server_SkipsAuthorization(t *testing.T) {
+	server := &Server{}
+	assert.True(t, server.SkipsAuthorization(context.Background()))
+}

internal/server/metadata/server_test.go

@@ -0,0 +1,13 @@
+package metadata
+
+import (
+	"context"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func Test_Server_SkipsAuthorization(t *testing.T) {
+	server := &Server{}
+	assert.True(t, server.SkipsAuthorization(context.Background()))
+}

internal/server/middleware/grpc/middleware.go

@@ -239,21 +239,21 @@ func EvaluationUnaryInterceptor(analyticsEnabled bool) grpc.UnaryServerIntercept
 // AuditEventUnaryInterceptor captures events and adds them to the trace span to be consumed downstream.
 func AuditEventUnaryInterceptor(logger *zap.Logger, eventPairChecker audit.EventPairChecker) grpc.UnaryServerInterceptor {
 	return func(ctx context.Context, req interface{}, info *grpc.UnaryServerInfo, handler grpc.UnaryHandler) (interface{}, error) {
-		var request flipt.Request
+		var requests []flipt.Request
 		r, ok := req.(flipt.Requester)
 
 		if !ok {
 			return handler(ctx, req)
 		}
 
-		request = r.Request()
+		requests = r.Request()
 
-		var event *audit.Event
+		var events []*audit.Event
 
 		actor := authn.ActorFromContext(ctx)
 
 		defer func() {
-			if event != nil {
+			for _, event := range events {
 				eventPair := fmt.Sprintf("%s:%s", event.Type, event.Action)
 
 				exists := eventPairChecker.Check(eventPair)
@@ -265,51 +265,50 @@ func AuditEventUnaryInterceptor(logger *zap.Logger, eventPairChecker audit.Event
 		}()
 
 		resp, err := handler(ctx, req)
-		if err != nil {
-			var uerr errs.ErrUnauthorized
-			if errors.As(err, &uerr) {
-				request.Status = flipt.StatusDenied
-				event = audit.NewEvent(request, actor, nil)
+		for _, request := range requests {
+			if err != nil {
+				var uerr errs.ErrUnauthorized
+				if errors.As(err, &uerr) {
+					request.Status = flipt.StatusDenied
+					events = append(events, audit.NewEvent(request, actor, nil))
+				}
+
+				continue
+			}
+
+			// Delete and Order request(s) have to be handled separately because they do not
+			// return the concrete type but rather an *empty.Empty response.
+			if request.Action == flipt.ActionDelete {
+				events = append(events, audit.NewEvent(request, actor, r))
+				continue
 			}
-			return resp, err
-		}
 
-		// Delete and Order request(s) have to be handled separately because they do not
-		// return the concrete type but rather an *empty.Empty response.
-		if request.Action == flipt.ActionDelete {
-			event = audit.NewEvent(request, actor, r)
-		} else {
 			switch r := req.(type) {
 			case *flipt.OrderRulesRequest, *flipt.OrderRolloutsRequest:
-				event = audit.NewEvent(request, actor, r)
+				events = append(events, audit.NewEvent(request, actor, r))
+				continue
 			}
-		}
 
-		// Short circuiting the middleware here since we have a non-nil event from
-		// detecting a delete.
-		if event != nil {
-			return resp, err
-		}
-
-		switch r := resp.(type) {
-		case *flipt.Flag:
-			event = audit.NewEvent(request, actor, audit.NewFlag(r))
-		case *flipt.Variant:
-			event = audit.NewEvent(request, actor, audit.NewVariant(r))
-		case *flipt.Segment:
-			event = audit.NewEvent(request, actor, audit.NewSegment(r))
-		case *flipt.Distribution:
-			event = audit.NewEvent(request, actor, audit.NewDistribution(r))
-		case *flipt.Constraint:
-			event = audit.NewEvent(request, actor, audit.NewConstraint(r))
-		case *flipt.Namespace:
-			event = audit.NewEvent(request, actor, audit.NewNamespace(r))
-		case *flipt.Rollout:
-			event = audit.NewEvent(request, actor, audit.NewRollout(r))
-		case *flipt.Rule:
-			event = audit.NewEvent(request, actor, audit.NewRule(r))
-		case *auth.CreateTokenResponse:
-			event = audit.NewEvent(request, actor, r.Authentication.Metadata)
+			switch r := resp.(type) {
+			case *flipt.Flag:
+				events = append(events, audit.NewEvent(request, actor, audit.NewFlag(r)))
+			case *flipt.Variant:
+				events = append(events, audit.NewEvent(request, actor, audit.NewVariant(r)))
+			case *flipt.Segment:
+				events = append(events, audit.NewEvent(request, actor, audit.NewSegment(r)))
+			case *flipt.Distribution:
+				events = append(events, audit.NewEvent(request, actor, audit.NewDistribution(r)))
+			case *flipt.Constraint:
+				events = append(events, audit.NewEvent(request, actor, audit.NewConstraint(r)))
+			case *flipt.Namespace:
+				events = append(events, audit.NewEvent(request, actor, audit.NewNamespace(r)))
+			case *flipt.Rollout:
+				events = append(events, audit.NewEvent(request, actor, audit.NewRollout(r)))
+			case *flipt.Rule:
+				events = append(events, audit.NewEvent(request, actor, audit.NewRule(r)))
+			case *auth.CreateTokenResponse:
+				events = append(events, audit.NewEvent(request, actor, r.Authentication.Metadata))
+			}
 		}
 
 		return resp, err

internal/server/ofrep/server.go

@@ -56,3 +56,7 @@ func New(logger *zap.Logger, cacheCfg config.CacheConfig, bridge Bridge) *Server
 func (s *Server) RegisterGRPC(server *grpc.Server) {
 	ofrep.RegisterOFREPServiceServer(server, s)
 }
+
+func (s *Server) SkipsAuthorization(ctx context.Context) bool {
+	return true
+}

internal/server/ofrep/server_test.go

@@ -0,0 +1,13 @@
+package ofrep
+
+import (
+	"context"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func Test_Server_SkipsAuthorization(t *testing.T) {
+	server := &Server{}
+	assert.True(t, server.SkipsAuthorization(context.Background()))
+}

rpc/flipt/auth/request.go

@@ -4,18 +4,18 @@ import (
 	"go.flipt.io/flipt/rpc/flipt"
 )
 
-func (req *CreateTokenRequest) Request() flipt.Request {
-	return flipt.NewRequest(flipt.ResourceAuthentication, flipt.ActionCreate, flipt.WithSubject(flipt.SubjectToken))
+func (req *CreateTokenRequest) Request() []flipt.Request {
+	return []flipt.Request{flipt.NewRequest(flipt.ResourceAuthentication, flipt.ActionCreate, flipt.WithSubject(flipt.SubjectToken))}
 }
 
-func (req *ListAuthenticationsRequest) Request() flipt.Request {
-	return flipt.NewRequest(flipt.ResourceAuthentication, flipt.ActionRead)
+func (req *ListAuthenticationsRequest) Request() []flipt.Request {
+	return []flipt.Request{flipt.NewRequest(flipt.ResourceAuthentication, flipt.ActionRead)}
 }
 
-func (req *GetAuthenticationRequest) Request() flipt.Request {
-	return flipt.NewRequest(flipt.ResourceAuthentication, flipt.ActionRead)
+func (req *GetAuthenticationRequest) Request() []flipt.Request {
+	return []flipt.Request{flipt.NewRequest(flipt.ResourceAuthentication, flipt.ActionRead)}
 }
 
-func (req *DeleteAuthenticationRequest) Request() flipt.Request {
-	return flipt.NewRequest(flipt.ResourceAuthentication, flipt.ActionDelete)
+func (req *DeleteAuthenticationRequest) Request() []flipt.Request {
+	return []flipt.Request{flipt.NewRequest(flipt.ResourceAuthentication, flipt.ActionDelete)}
 }