[#82] router: Make IsAllow receive client defined overrides

* Change `ChainRouter` interface: `IsAllow` should also receive
  client defined overrides that are checked after local overrides
  but before morph chains. Client defined overrides are checked for
  per request.
* Fix code that uses `ChainRouter`.
* Fix unit-tests.

Signed-off-by: Airat Arifullin <aarifullin@yadro.com>

iam/converter_test.go

@@ -717,13 +717,13 @@ func TestIPConditions(t *testing.T) {
 
 		req := testutil.NewRequest("s3:CreateBucket", testutil.NewResource(fmt.Sprintf(s3.ResourceFormatS3Bucket, "bkt"), nil),
 			map[string]string{common.PropertyKeyFrostFSSourceIP: "203.0.113.128"})
-		status, _, err := s.IsAllowed(chain.S3, engine.NewRequestTargetWithNamespace(""), req)
+		status, _, err := s.IsAllowed(chain.S3, engine.NewRequestTargetWithNamespace(""), req, engine.NoClientDefined)
 		require.NoError(t, err)
 		require.Equal(t, chain.Allow.String(), status.String())
 
 		req = testutil.NewRequest("s3:CreateBucket", testutil.NewResource(fmt.Sprintf(s3.ResourceFormatS3Bucket, "bkt"), nil),
 			map[string]string{common.PropertyKeyFrostFSSourceIP: "203.0.114.0"})
-		status, _, err = s.IsAllowed(chain.S3, engine.NewRequestTargetWithNamespace(""), req)
+		status, _, err = s.IsAllowed(chain.S3, engine.NewRequestTargetWithNamespace(""), req, engine.NoClientDefined)
 		require.NoError(t, err)
 		require.Equal(t, chain.NoRuleFound.String(), status.String())
 	})
@@ -1092,7 +1092,7 @@ func TestComplexNativeConditions(t *testing.T) {
 	} {
 		t.Run(tc.name, func(t *testing.T) {
 			req := testutil.NewRequest(tc.action, testutil.NewResource(tc.resource, tc.resourceMap), tc.requestMap)
-			status, _, err := s.IsAllowed("name", engine.NewRequestTargetWithNamespace("ns"), req)
+			status, _, err := s.IsAllowed("name", engine.NewRequestTargetWithNamespace("ns"), req, engine.NoClientDefined)
 			require.NoError(t, err)
 			require.Equal(t, tc.status.String(), status.String())
 		})
@@ -1324,7 +1324,7 @@ func TestComplexS3Conditions(t *testing.T) {
 	} {
 		t.Run(tc.name, func(t *testing.T) {
 			req := testutil.NewRequest(tc.action, testutil.NewResource(tc.resource, tc.resourceMap), tc.requestMap)
-			status, _, err := s.IsAllowed("name", engine.NewRequestTargetWithNamespace("ns"), req)
+			status, _, err := s.IsAllowed("name", engine.NewRequestTargetWithNamespace("ns"), req, engine.NoClientDefined)
 			require.NoError(t, err)
 			require.Equal(t, tc.status.String(), status.String())
 		})
@@ -1365,19 +1365,19 @@ func TestS3BucketResource(t *testing.T) {
 
 	// check we match just "bucket1" resource
 	req := testutil.NewRequest("s3:HeadBucket", testutil.NewResource(fmt.Sprintf(s3.ResourceFormatS3Bucket, bktName1), nil), nil)
-	status, _, err := s.IsAllowed(chainName, engine.NewRequestTargetWithNamespace(namespace), req)
+	status, _, err := s.IsAllowed(chainName, engine.NewRequestTargetWithNamespace(namespace), req, engine.NoClientDefined)
 	require.NoError(t, err)
 	require.Equal(t, chain.AccessDenied.String(), status.String())
 
 	// check we match just "bucket2" resource
 	req = testutil.NewRequest("s3:HeadBucket", testutil.NewResource(fmt.Sprintf(s3.ResourceFormatS3Bucket, bktName2), nil), nil)
-	status, _, err = s.IsAllowed(chainName, engine.NewRequestTargetWithNamespace(namespace), req)
+	status, _, err = s.IsAllowed(chainName, engine.NewRequestTargetWithNamespace(namespace), req, engine.NoClientDefined)
 	require.NoError(t, err)
 	require.Equal(t, chain.Allow.String(), status.String())
 
 	// check we also match "bucket2/object" resource
 	req = testutil.NewRequest("s3:PutObject", testutil.NewResource(fmt.Sprintf(s3.ResourceFormatS3BucketObject, bktName2, "object"), nil), nil)
-	status, _, err = s.IsAllowed(chainName, engine.NewRequestTargetWithNamespace(namespace), req)
+	status, _, err = s.IsAllowed(chainName, engine.NewRequestTargetWithNamespace(namespace), req, engine.NoClientDefined)
 	require.NoError(t, err)
 	require.Equal(t, chain.Allow.String(), status.String())
 }

iam/policy_test.go

@@ -544,7 +544,7 @@ func TestProcessDenyFirst(t *testing.T) {
 	resource := testutil.NewResource("arn:aws:s3:::test-bucket/object", nil)
 	request := testutil.NewRequest("s3:PutObject", resource, map[string]string{s3.PropertyKeyOwner: mockResolver.users["root/user-name"]})
 
-	status, found, err := s.IsAllowed(chain.S3, engine.NewRequestTarget("ns", ""), request)
+	status, found, err := s.IsAllowed(chain.S3, engine.NewRequestTarget("ns", ""), request, engine.NoClientDefined)
 	require.NoError(t, err)
 	require.True(t, found)
 	require.Equal(t, chain.AccessDenied, status)

pkg/engine/chain_router.go

@@ -1,6 +1,8 @@
 package engine
 
 import (
+	"fmt"
+
 	"git.frostfs.info/TrueCloudLab/policy-engine/pkg/chain"
 	"git.frostfs.info/TrueCloudLab/policy-engine/pkg/resource"
 )
@@ -24,13 +26,21 @@ func NewDefaultChainRouterWithLocalOverrides(morph MorphRuleChainStorageReader,
 	}
 }
 
-func (dr *defaultChainRouter) IsAllowed(name chain.Name, rt RequestTarget, r resource.Request) (status chain.Status, ruleFound bool, err error) {
+func (dr *defaultChainRouter) IsAllowed(name chain.Name, rt RequestTarget, r resource.Request, clientDefined ClientDefinedOverrides) (status chain.Status, ruleFound bool, err error) {
 	status, ruleFound, err = dr.checkLocal(name, rt, r)
 	if err != nil {
 		return chain.NoRuleFound, false, err
 	} else if ruleFound {
 		// The local overrides have the highest priority and thus
-		// morph rules are not considered if a local one is found.
+		// morph rules and client overrides are not considered if a local one is found.
+		return
+	}
+
+	status, ruleFound, err = checkClientDefined(name, rt, r, clientDefined)
+	if err != nil {
+		return chain.NoRuleFound, false, fmt.Errorf("client defined: %w", err)
+	} else if ruleFound {
+		// The client defined overrides have the higher priority then morph chains.
 		return
 	}
 
@@ -61,6 +71,29 @@ func (dr *defaultChainRouter) checkLocal(name chain.Name, rt RequestTarget, r re
 	return
 }
 
+func checkClientDefined(name chain.Name, rt RequestTarget, r resource.Request, clientDefined ClientDefinedOverrides) (status chain.Status, ruleFound bool, err error) {
+	if clientDefined == NoClientDefined {
+		return
+	}
+	var ruleFounds []bool
+	for _, target := range rt.Targets() {
+		status, ruleFound, err = matchClientDefined(name, target, r, clientDefined)
+		if err != nil || ruleFound && status != chain.Allow {
+			return
+		}
+		ruleFounds = append(ruleFounds, ruleFound)
+	}
+
+	status = chain.NoRuleFound
+	for _, ruleFound = range ruleFounds {
+		if ruleFound {
+			status = chain.Allow
+			break
+		}
+	}
+	return
+}
+
 func (dr *defaultChainRouter) checkMorph(name chain.Name, rt RequestTarget, r resource.Request) (status chain.Status, ruleFound bool, err error) {
 	var ruleFounds []bool
 	for _, target := range rt.Targets() {
@@ -86,7 +119,16 @@ func (dr *defaultChainRouter) matchLocalOverrides(name chain.Name, target Target
 	if err != nil {
 		return
 	}
-	status, ruleFound = dr.getStatusFromChains(localOverrides, r)
+	status, ruleFound = getStatusFromChains(localOverrides, r)
+	return
+}
+
+func matchClientDefined(name chain.Name, target Target, r resource.Request, clientDefined ClientDefinedOverrides) (status chain.Status, ruleFound bool, err error) {
+	overrides, err := clientDefined.ListOverrides(name, target)
+	if err != nil {
+		return
+	}
+	status, ruleFound = getStatusFromChains(overrides, r)
 	return
 }
 
@@ -95,11 +137,11 @@ func (dr *defaultChainRouter) matchMorphRuleChains(name chain.Name, target Targe
 	if err != nil {
 		return chain.NoRuleFound, false, err
 	}
-	status, ruleFound = dr.getStatusFromChains(namespaceChains, r)
+	status, ruleFound = getStatusFromChains(namespaceChains, r)
 	return
 }
 
-func (dr *defaultChainRouter) getStatusFromChains(chains []*chain.Chain, r resource.Request) (chain.Status, bool) {
+func getStatusFromChains(chains []*chain.Chain, r resource.Request) (chain.Status, bool) {
 	var allow bool
 	for _, c := range chains {
 		if status, found := c.Match(r); found {

pkg/engine/inmemory/inmemory.go

@@ -43,6 +43,6 @@ func (im *inmemory) MorphRuleChainStorage() engine.MorphRuleChainStorage {
 	return im.morph
 }
 
-func (im *inmemory) IsAllowed(name chain.Name, rt engine.RequestTarget, r resource.Request) (status chain.Status, ruleFound bool, err error) {
-	return im.router.IsAllowed(name, rt, r)
+func (im *inmemory) IsAllowed(name chain.Name, rt engine.RequestTarget, r resource.Request, clientDefined engine.ClientDefinedOverrides) (status chain.Status, ruleFound bool, err error) {
+	return im.router.IsAllowed(name, rt, r, clientDefined)
 }

pkg/engine/inmemory/inmemory_test.go

@@ -51,7 +51,7 @@ func TestInmemory(t *testing.T) {
 		"Actor":    actor1,
 	})
 
-	status, ok, _ := s.IsAllowed(chain.Ingress, engine.NewRequestTargetWithNamespace(namespace), reqGood)
+	status, ok, _ := s.IsAllowed(chain.Ingress, engine.NewRequestTargetWithNamespace(namespace), reqGood, engine.NoClientDefined)
 	require.Equal(t, chain.NoRuleFound, status)
 	require.False(t, ok)
 
@@ -141,7 +141,59 @@ func TestInmemory(t *testing.T) {
 			"SourceIP": "10.122.1.20",
 			"Actor":    actor1,
 		})
-		status, ok, _ := s.IsAllowed(chain.Ingress, engine.NewRequestTarget(namespace, container), reqBadIP)
+		status, ok, _ := s.IsAllowed(chain.Ingress, engine.NewRequestTarget(namespace, container), reqBadIP, engine.NoClientDefined)
+		require.Equal(t, chain.AccessDenied, status)
+		require.True(t, ok)
+	})
+	t.Run("allow by client defined then deny by local overrides", func(t *testing.T) {
+		reqBadIP := resourcetest.NewRequest("native::object::put", res, map[string]string{
+			"SourceIP": "10.122.1.20",
+			"Actor":    actor1,
+		})
+
+		clientDef := NewInMemoryLocalOverrides()
+		clientDef.LocalStorage().AddOverride(chain.Ingress, engine.ContainerTarget(container), &chain.Chain{
+			Rules: []chain.Rule{
+				{
+					Status:    chain.Allow,
+					Actions:   chain.Actions{Names: []string{"native::object::put"}},
+					Resources: chain.Resources{Names: []string{"native::object::abc/*"}},
+					Condition: []chain.Condition{
+						{
+							Op:    chain.CondStringEquals,
+							Kind:  chain.KindRequest,
+							Key:   "SourceIP",
+							Value: "10.122.1.20",
+						},
+					},
+				},
+			},
+		})
+
+		status, ok, _ := s.IsAllowed(chain.Ingress, engine.NewRequestTarget(namespace, container), reqBadIP, clientDef.LocalStorage())
+		require.Equal(t, chain.Allow, status)
+		require.True(t, ok)
+
+		s.LocalStorage().AddOverride(chain.Ingress, engine.ContainerTarget(container), &chain.Chain{
+			Rules: []chain.Rule{
+				{
+					Status:    chain.AccessDenied,
+					Actions:   chain.Actions{Names: []string{"native::object::put"}},
+					Resources: chain.Resources{Names: []string{"native::object::abc/*"}},
+					Condition: []chain.Condition{
+						{
+							Op:    chain.CondStringEquals,
+							Kind:  chain.KindRequest,
+							Key:   "SourceIP",
+							Value: "10.122.1.20",
+						},
+					},
+				},
+			},
+		})
+
+		status, ok, _ = s.IsAllowed(chain.Ingress, engine.NewRequestTarget(namespace, container), reqBadIP, clientDef.LocalStorage())
+		// Local overrides have higher priority then client defined overrides.
 		require.Equal(t, chain.AccessDenied, status)
 		require.True(t, ok)
 	})
@@ -151,7 +203,7 @@ func TestInmemory(t *testing.T) {
 			"SourceIP": "10.1.1.13",
 			"Actor":    actor2,
 		})
-		status, ok, _ := s.IsAllowed(chain.Ingress, engine.NewRequestTarget(namespace, container), reqBadActor)
+		status, ok, _ := s.IsAllowed(chain.Ingress, engine.NewRequestTarget(namespace, container), reqBadActor, engine.NoClientDefined)
 		require.Equal(t, chain.AccessDenied, status)
 		require.True(t, ok)
 	})
@@ -162,14 +214,14 @@ func TestInmemory(t *testing.T) {
 		status, ok, _ := s.IsAllowed(chain.Ingress, engine.NewRequestTarget(namespace, container), resourcetest.NewRequest("native::object::get", objGood, map[string]string{
 			"SourceIP": "10.1.1.14",
 			"Actor":    actor2,
-		}))
+		}), engine.NoClientDefined)
 		require.Equal(t, chain.Allow, status)
 		require.True(t, ok)
 
 		status, ok, _ = s.IsAllowed(chain.Ingress, engine.NewRequestTarget(namespace, container), resourcetest.NewRequest("native::object::get", objBadAttr, map[string]string{
 			"SourceIP": "10.1.1.14",
 			"Actor":    actor2,
-		}))
+		}), engine.NoClientDefined)
 		require.Equal(t, chain.NoRuleFound, status)
 		require.False(t, ok)
 	})
@@ -179,28 +231,28 @@ func TestInmemory(t *testing.T) {
 			"SourceIP": "10.1.1.12",
 			"Actor":    actor1,
 		})
-		status, ok, _ := s.IsAllowed(chain.Ingress, engine.NewRequestTarget(namespace, container), reqBadOperation)
+		status, ok, _ := s.IsAllowed(chain.Ingress, engine.NewRequestTarget(namespace, container), reqBadOperation, engine.NoClientDefined)
 		require.Equal(t, chain.AccessDenied, status)
 		require.True(t, ok)
 	})
 	t.Run("inverted rules", func(t *testing.T) {
 		req := resourcetest.NewRequest("native::object::put", resourcetest.NewResource(object, nil), nil)
-		status, ok, _ = s.IsAllowed(chain.Ingress, engine.NewRequestTarget(namespace2, container), req)
+		status, ok, _ = s.IsAllowed(chain.Ingress, engine.NewRequestTarget(namespace2, container), req, engine.NoClientDefined)
 		require.Equal(t, chain.NoRuleFound, status)
 		require.False(t, ok)
 
 		req = resourcetest.NewRequest("native::object::put", resourcetest.NewResource("native::object::cba/def", nil), nil)
-		status, ok, _ = s.IsAllowed(chain.Ingress, engine.NewRequestTarget(namespace2, container), req)
+		status, ok, _ = s.IsAllowed(chain.Ingress, engine.NewRequestTarget(namespace2, container), req, engine.NoClientDefined)
 		require.Equal(t, chain.AccessDenied, status)
 		require.True(t, ok)
 
 		req = resourcetest.NewRequest("native::object::get", resourcetest.NewResource("native::object::cba/def", nil), nil)
-		status, ok, _ = s.IsAllowed(chain.Ingress, engine.NewRequestTarget(namespace2, container), req)
+		status, ok, _ = s.IsAllowed(chain.Ingress, engine.NewRequestTarget(namespace2, container), req, engine.NoClientDefined)
 		require.Equal(t, chain.NoRuleFound, status)
 		require.False(t, ok)
 	})
 	t.Run("good", func(t *testing.T) {
-		status, ok, _ = s.IsAllowed(chain.Ingress, engine.NewRequestTarget(namespace, container), reqGood)
+		status, ok, _ = s.IsAllowed(chain.Ingress, engine.NewRequestTarget(namespace, container), reqGood, engine.NoClientDefined)
 		require.Equal(t, chain.NoRuleFound, status)
 		require.False(t, ok)
 
@@ -221,7 +273,7 @@ func TestInmemory(t *testing.T) {
 			require.NoError(t, err)
 			itemStacksEqual(t, it.Values, toStackItems(container))
 
-			status, ok, _ = s.IsAllowed(chain.Ingress, engine.NewRequestTarget(namespace, container), reqGood)
+			status, ok, _ = s.IsAllowed(chain.Ingress, engine.NewRequestTarget(namespace, container), reqGood, engine.NoClientDefined)
 			require.Equal(t, chain.NoRuleFound, status)
 			require.False(t, ok)
 		})
@@ -244,7 +296,7 @@ func TestInmemory(t *testing.T) {
 			require.NoError(t, err)
 			itemStacksEqual(t, it.Values, toStackItems(container))
 
-			status, ok, _ = s.IsAllowed(chain.Ingress, engine.NewRequestTarget(namespace, container), reqGood)
+			status, ok, _ = s.IsAllowed(chain.Ingress, engine.NewRequestTarget(namespace, container), reqGood, engine.NoClientDefined)
 			require.Equal(t, chain.QuotaLimitReached, status)
 			require.True(t, ok)
 		})
@@ -260,7 +312,7 @@ func TestInmemory(t *testing.T) {
 			require.NoError(t, err)
 			itemStacksEqual(t, it.Values, toStackItems(container))
 
-			status, ok, _ = s.IsAllowed(chain.Ingress, engine.NewRequestTarget(namespace, container), reqGood)
+			status, ok, _ = s.IsAllowed(chain.Ingress, engine.NewRequestTarget(namespace, container), reqGood, engine.NoClientDefined)
 			require.Equal(t, chain.NoRuleFound, status)
 			require.False(t, ok)
 		})

pkg/engine/interface.go

@@ -8,10 +8,27 @@ import (
 	"github.com/nspcc-dev/neo-go/pkg/util"
 )
 
+type ClientDefinedOverrides interface {
+	ListOverrides(name chain.Name, target Target) ([]*chain.Chain, error)
+}
+
+type noClientDefinedImpl struct{}
+
+func (n *noClientDefinedImpl) ListOverrides(_ chain.Name, _ Target) ([]*chain.Chain, error) {
+	return []*chain.Chain{}, nil
+}
+
+var NoClientDefined ClientDefinedOverrides = &noClientDefinedImpl{}
+
 type ChainRouter interface {
 	// IsAllowed returns status for the operation after all checks.
 	// The second return value signifies whether a matching rule was found.
-	IsAllowed(name chain.Name, reqTarget RequestTarget, r resource.Request) (status chain.Status, found bool, err error)
+	//
+	// IsAllowed can be invoked with overrides defined by a client to check the request.
+	// This check is performed after local overrides but before checking of chains defined
+	// in morph storage.
+	// If a client defines no extra chains, NoClientDefined should be passed.
+	IsAllowed(name chain.Name, reqTarget RequestTarget, r resource.Request, clientDefined ClientDefinedOverrides) (status chain.Status, found bool, err error)
 }
 
 // LocalOverrideStorage is the interface to manage local overrides defined