Have only auth accessing actions happen after auth

Signed-off-by: Alex Snaps <[email protected]>
Kuadrant · Nov 14, 2024 · e6ef741 · e6ef741
1 parent fe39f7f
commit e6ef741
Show file tree

Hide file tree

Showing 4 changed files with 134 additions and 2 deletions.
diff --git a/controllers/envoy_gateway_extension_reconciler.go b/controllers/envoy_gateway_extension_reconciler.go
@@ -185,7 +185,13 @@ func (r *EnvoyGatewayExtensionReconciler) buildWasmConfigs(ctx context.Context,
 
 		// rate limit
 		if effectivePolicy, ok := effectiveRateLimitPoliciesMap[pathID]; ok {
-			actions = append(actions, buildWasmActionsForRateLimit(effectivePolicy, state)...)
+			rlAction := buildWasmActionsForRateLimit(effectivePolicy, state)
+			if hasAuthAccess(rlAction) {
+				actions = append(actions, rlAction...)
+			} else {
+				// pre auth rate limiting
+				actions = append(rlAction, actions...)
+			}
 		}
 
 		if len(actions) == 0 {

diff --git a/controllers/istio_extension_reconciler.go b/controllers/istio_extension_reconciler.go
@@ -188,7 +188,13 @@ func (r *IstioExtensionReconciler) buildWasmConfigs(ctx context.Context, state *
 
 		// rate limit
 		if effectivePolicy, ok := effectiveRateLimitPoliciesMap[pathID]; ok {
-			actions = append(actions, buildWasmActionsForRateLimit(effectivePolicy, state)...)
+			rlAction := buildWasmActionsForRateLimit(effectivePolicy, state)
+			if hasAuthAccess(rlAction) {
+				actions = append(actions, rlAction...)
+			} else {
+				// pre auth rate limiting
+				actions = append(rlAction, actions...)
+			}
 		}
 
 		if len(actions) == 0 {
@@ -212,6 +218,15 @@ func (r *IstioExtensionReconciler) buildWasmConfigs(ctx context.Context, state *
 	return wasmConfigs, nil
 }
 
+func hasAuthAccess(actionSet []wasm.Action) bool {
+	for _, action := range actionSet {
+		if action.HasAuthAccess() {
+			return true
+		}
+	}
+	return false
+}
+
 // buildIstioWasmPluginForGateway builds a desired WasmPlugin custom resource for a given gateway and corresponding wasm config
 func buildIstioWasmPluginForGateway(gateway *machinery.Gateway, wasmConfig wasm.Config) *istioclientgoextensionv1alpha1.WasmPlugin {
 	wasmPlugin := &istioclientgoextensionv1alpha1.WasmPlugin{

diff --git a/pkg/wasm/types.go b/pkg/wasm/types.go
@@ -4,6 +4,7 @@ import (
 	"bytes"
 	"encoding/json"
 	"errors"
+	"strings"
 
 	_struct "google.golang.org/protobuf/types/known/structpb"
 	apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
@@ -144,6 +145,26 @@ type Action struct {
 	Data []DataType `json:"data,omitempty"`
 }
 
+func (a *Action) HasAuthAccess() bool {
+	for _, predicate := range a.Predicates {
+		if strings.Contains(predicate, "auth.") {
+			return true
+		}
+	}
+	for _, data := range a.Data {
+		switch val := data.Value.(type) {
+		case *Static:
+
+			continue
+		case *Expression:
+			if strings.Contains(val.ExpressionItem.Value, "auth.") {
+				return true
+			}
+		}
+	}
+	return false
+}
+
 func (a *Action) EqualTo(other Action) bool {
 	if a.Scope != other.Scope ||
 		a.ServiceName != other.ServiceName ||

diff --git a/pkg/wasm/types_test.go b/pkg/wasm/types_test.go
@@ -236,3 +236,93 @@ data:
 		})
 	}
 }
+
+func TestAuthAccesses(t *testing.T) {
+	action := Action{
+		ServiceName: "ratelimit-service",
+		Scope:       "default/other",
+		Predicates: []string{
+			"source.address != '127.0.0.1'",
+		},
+		Data: []DataType{
+			{
+				Value: &Static{
+					Static: StaticSpec{
+						Key:   "limit.global__f63bec56",
+						Value: "1",
+					},
+				},
+			},
+		},
+	}
+
+	if action.HasAuthAccess() {
+		t.Fatal("must not have auth access")
+	}
+
+	action = Action{
+		ServiceName: "ratelimit-service",
+		Scope:       "default/other",
+		Predicates: []string{
+			"auth.something != '127.0.0.1'",
+		},
+		Data: []DataType{
+			{
+				Value: &Static{
+					Static: StaticSpec{
+						Key:   "limit.global__f63bec56",
+						Value: "1",
+					},
+				},
+			},
+		},
+	}
+
+	if !action.HasAuthAccess() {
+		t.Fatal("must have auth access")
+	}
+
+	action = Action{
+		ServiceName: "ratelimit-service",
+		Scope:       "default/other",
+		Predicates: []string{
+			"source.address != '127.0.0.1'",
+		},
+		Data: []DataType{
+			{
+				Value: &Expression{
+					ExpressionItem: ExpressionItem{
+						Key:   "limit.global__f63bec56",
+						Value: "auth.identity.anonymous",
+					},
+				},
+			},
+		},
+	}
+
+	if !action.HasAuthAccess() {
+		t.Fatal("must have auth access")
+	}
+
+	action = Action{
+		ServiceName: "ratelimit-service",
+		Scope:       "default/other",
+		Predicates: []string{
+			"source.address != '127.0.0.1'",
+		},
+		Data: []DataType{
+			{
+				Value: &Static{
+					Static: StaticSpec{
+						Key:   "auth.global__f63bec56",
+						Value: "auth",
+					},
+				},
+			},
+		},
+	}
+
+	if action.HasAuthAccess() {
+		t.Fatal("must not have auth access")
+	}
+}