From 1375e8f7fdc44d4c552b4d67a0f6ef36edff1a7b Mon Sep 17 00:00:00 2001
From: Dmitrii Stepanov <d.stepanov@yadro.com>
Date: Fri, 8 Dec 2023 12:37:29 +0300
Subject: [PATCH] [#21] router: Make Deny the highest priority

Signed-off-by: Dmitrii Stepanov <d.stepanov@yadro.com>
---
 iam/policy_test.go         | 72 ++++++++++++++++++++++++++++++++++++++
 pkg/engine/chain_router.go | 30 ++++++++++------
 2 files changed, 91 insertions(+), 11 deletions(-)

diff --git a/iam/policy_test.go b/iam/policy_test.go
index 6172655..046035d 100644
--- a/iam/policy_test.go
+++ b/iam/policy_test.go
@@ -2,8 +2,14 @@ package iam
 
 import (
 	"encoding/json"
+	"fmt"
 	"testing"
 
+	"git.frostfs.info/TrueCloudLab/policy-engine/pkg/chain"
+	"git.frostfs.info/TrueCloudLab/policy-engine/pkg/engine"
+	"git.frostfs.info/TrueCloudLab/policy-engine/pkg/engine/inmemory"
+	"git.frostfs.info/TrueCloudLab/policy-engine/pkg/resource/testutil"
+	"git.frostfs.info/TrueCloudLab/policy-engine/schema/native"
 	"github.com/stretchr/testify/require"
 )
 
@@ -428,3 +434,69 @@ func TestValidatePolicies(t *testing.T) {
 		})
 	}
 }
+
+func TestProcessDenyFirst(t *testing.T) {
+	identityBasedPolicyStr := `
+{
+    "Version": "2012-10-17",
+    "Statement": [
+        {
+            "Effect": "Allow",
+			"Principal": {
+				"AWS": [ "arn:aws:iam::root:user/user-name" ]
+			},
+            "Action": ["s3:PutObject" ],
+            "Resource": "arn:aws:s3:::*"
+        }
+    ]
+}
+`
+
+	resourceBasedPolicyStr := `
+{
+    "Version": "2012-10-17",
+    "Statement": [
+        {
+            "Effect": "Deny",
+            "Principal": "*",
+            "Action": "s3:*",
+            "Resource": [ "arn:aws:s3:::test-bucket/*" ]
+        }
+    ]
+}
+`
+
+	var identityPolicy Policy
+	err := json.Unmarshal([]byte(identityBasedPolicyStr), &identityPolicy)
+	require.NoError(t, err)
+
+	var resourcePolicy Policy
+	err = json.Unmarshal([]byte(resourceBasedPolicyStr), &resourcePolicy)
+	require.NoError(t, err)
+
+	mockResolver := newMockUserResolver([]string{"root/user-name"}, []string{"test-bucket"})
+
+	identityNativePolicy, err := ConvertToNativeChain(identityPolicy, mockResolver)
+	require.NoError(t, err)
+
+	resourceNativePolicy, err := ConvertToNativeChain(resourcePolicy, mockResolver)
+	require.NoError(t, err)
+
+	s := inmemory.NewInMemory()
+
+	target := engine.NamespaceTarget("ns")
+
+	_, _, err = s.MorphRuleChainStorage().AddMorphRuleChain(chain.Ingress, target, identityNativePolicy)
+	require.NoError(t, err)
+
+	_, _, err = s.MorphRuleChainStorage().AddMorphRuleChain(chain.Ingress, target, resourceNativePolicy)
+	require.NoError(t, err)
+
+	resource := testutil.NewResource(fmt.Sprintf(native.ResourceFormatRootContainerObjects, mockResolver.buckets["test-bucket"]), nil)
+	request := testutil.NewRequest("PutObject", resource, map[string]string{native.PropertyKeyActorPublicKey: mockResolver.users["root/user-name"]})
+
+	status, found, err := s.IsAllowed(chain.Ingress, engine.NewRequestTarget("ns", ""), request)
+	require.NoError(t, err)
+	require.True(t, found)
+	require.Equal(t, chain.AccessDenied, status)
+}
diff --git a/pkg/engine/chain_router.go b/pkg/engine/chain_router.go
index c82f753..93775b8 100644
--- a/pkg/engine/chain_router.go
+++ b/pkg/engine/chain_router.go
@@ -86,23 +86,31 @@ func (dr *defaultChainRouter) matchLocalOverrides(name chain.Name, target Target
 	if err != nil {
 		return
 	}
-	for _, c := range localOverrides {
-		if status, ruleFound = c.Match(r); ruleFound && status != chain.Allow {
-			return
-		}
-	}
+	status, ruleFound = dr.getStatusFromChains(localOverrides, r)
 	return
 }
 
 func (dr *defaultChainRouter) matchMorphRuleChains(name chain.Name, target Target, r resource.Request) (status chain.Status, ruleFound bool, err error) {
 	namespaceChains, err := dr.morph.ListMorphRuleChains(name, target)
 	if err != nil {
-		return
-	}
-	for _, c := range namespaceChains {
-		if status, ruleFound = c.Match(r); ruleFound {
-			return
-		}
+		return chain.NoRuleFound, false, err
 	}
+	status, ruleFound = dr.getStatusFromChains(namespaceChains, r)
 	return
 }
+
+func (dr *defaultChainRouter) getStatusFromChains(chains []*chain.Chain, r resource.Request) (chain.Status, bool) {
+	var allow bool
+	for _, c := range chains {
+		if status, found := c.Match(r); found {
+			if status != chain.Allow {
+				return status, true
+			}
+			allow = true
+		}
+	}
+	if allow {
+		return chain.Allow, true
+	}
+	return chain.NoRuleFound, false
+}