From 3df9afdb302ef281a889f5c5180aaf77038c4391 Mon Sep 17 00:00:00 2001
From: Evgenii Stratonikov <e.stratonikov@yadro.com>
Date: Tue, 20 Aug 2024 18:54:39 +0300
Subject: [PATCH] engine: Fix ruleFound return value

It can be false if the first targets allows operation and the last one
returns NoRuleFound.

Found by @mbiryukova .

Signed-off-by: Evgenii Stratonikov <e.stratonikov@yadro.com>
---
 pkg/engine/chain_router.go           | 10 +++---
 pkg/engine/inmemory/inmemory_test.go | 50 ++++++++++++++++++++++++++++
 2 files changed, 54 insertions(+), 6 deletions(-)

diff --git a/pkg/engine/chain_router.go b/pkg/engine/chain_router.go
index fe6c470..3eced4a 100644
--- a/pkg/engine/chain_router.go
+++ b/pkg/engine/chain_router.go
@@ -52,11 +52,10 @@ func (dr *defaultChainRouter) checkLocal(name chain.Name, rt RequestTarget, r re
 		hasAllow = hasAllow || ruleFound
 	}
 
-	status = chain.NoRuleFound
 	if hasAllow {
-		status = chain.Allow
+		return chain.Allow, true, nil
 	}
-	return
+	return chain.NoRuleFound, false, nil
 }
 
 func (dr *defaultChainRouter) checkMorph(name chain.Name, rt RequestTarget, r resource.Request) (status chain.Status, ruleFound bool, err error) {
@@ -69,11 +68,10 @@ func (dr *defaultChainRouter) checkMorph(name chain.Name, rt RequestTarget, r re
 		hasAllow = hasAllow || ruleFound
 	}
 
-	status = chain.NoRuleFound
 	if hasAllow {
-		status = chain.Allow
+		return chain.Allow, true, nil
 	}
-	return
+	return chain.NoRuleFound, false, nil
 }
 
 func (dr *defaultChainRouter) matchLocalOverrides(name chain.Name, target Target, r resource.Request) (status chain.Status, ruleFound bool, err error) {
diff --git a/pkg/engine/inmemory/inmemory_test.go b/pkg/engine/inmemory/inmemory_test.go
index fca6935..484750d 100644
--- a/pkg/engine/inmemory/inmemory_test.go
+++ b/pkg/engine/inmemory/inmemory_test.go
@@ -31,6 +31,49 @@ func TestAddRootOverrides(t *testing.T) {
 	require.Equal(t, string(id), string(res[0].ID))
 }
 
+func TestInmemory_MultipleTargets(t *testing.T) {
+	const op = "ape::test::op"
+
+	targets := []engine.Target{
+		engine.NamespaceTarget("ns1"),
+		engine.ContainerTarget("cnr1"),
+		engine.GroupTarget("group1"),
+		engine.UserTarget("user1"),
+	}
+	for _, tt := range targets {
+		t.Run("morph", func(t *testing.T) {
+			s := NewInMemoryLocalOverrides()
+			s.MorphRuleChainStorage().AddMorphRuleChain(chain.Ingress, tt, &chain.Chain{
+				Rules: []chain.Rule{{
+					Status:  chain.Allow,
+					Actions: chain.Actions{Names: []string{op}},
+				}},
+			})
+
+			req := resourcetest.NewRequest(op, resourcetest.NewResource("r", nil), nil)
+			status, found, err := s.IsAllowed(chain.Ingress, engine.NewRequestTargetExtended("ns1", "cnr1", "user1", []string{"group1"}), req)
+			require.NoError(t, err)
+			require.True(t, found)
+			require.Equal(t, chain.Allow, status)
+		})
+		t.Run("override", func(t *testing.T) {
+			s := NewInMemoryLocalOverrides()
+			s.LocalStorage().AddOverride(chain.Ingress, tt, &chain.Chain{
+				Rules: []chain.Rule{{
+					Status:  chain.Allow,
+					Actions: chain.Actions{Names: []string{op}},
+				}},
+			})
+
+			req := resourcetest.NewRequest(op, resourcetest.NewResource("r", nil), nil)
+			status, found, err := s.IsAllowed(chain.Ingress, engine.NewRequestTargetExtended("ns1", "cnr1", "user1", []string{"group1"}), req)
+			require.NoError(t, err)
+			require.True(t, found)
+			require.Equal(t, chain.Allow, status)
+		})
+	}
+}
+
 func TestInmemory(t *testing.T) {
 	const (
 		object     = "native::object::abc/xyz"
@@ -204,6 +247,13 @@ func TestInmemory(t *testing.T) {
 		require.Equal(t, chain.NoRuleFound, status)
 		require.False(t, ok)
 
+		status, ok, _ = s.IsAllowed(chain.Ingress, engine.NewRequestTarget(namespace, container), resourcetest.NewRequest("native::object::get", res, map[string]string{
+			"Department": "HR",
+			"Actor":      actor2,
+		}))
+		require.Equal(t, chain.Allow, status)
+		require.True(t, ok)
+
 		t.Run("quota on a different container", func(t *testing.T) {
 			s.LocalStorage().AddOverride(chain.Ingress, engine.ContainerTarget(container), &chain.Chain{
 				Rules: []chain.Rule{{