[TT-12897] Merge path based permissions when combining policies

internal/policy/Taskfile.yml

@@ -19,8 +19,24 @@ tasks:
       - defer: { task: services:down }
       - goimports -w .
       - go fmt ./...
+      - task: test
+        vars:
+          run: '{{.run}}'
+
+  test:
+    desc: "Run tests"
+    requires:
+      vars: [run]
+    cmds:
       - go test -count=1 -run='({{.run}})' -cover -coverprofile=pkg.cov -v .
 
+  stress:
+    desc: "Run stress tests"
+    requires:
+      vars: [run]
+    cmds:
+      - go test -count=2000 -run='({{.run}})' -cover -coverprofile=pkg.cov .
+
   cover:
     desc: "Show source coverage"
     aliases: [coverage, cov]

internal/policy/apply.go

@@ -22,6 +22,7 @@ type Repository interface {
 	PolicyByID(string) (user.Policy, bool)
 }
 
+// Service represents the policy service for gateway.
 type Service struct {
 	storage Repository
 	logger  *logrus.Logger
@@ -30,6 +31,7 @@ type Service struct {
 	orgID *string
 }
 
+// New creates a new policy.Service object.
 func New(orgID *string, storage Repository, logger *logrus.Logger) *Service {
 	return &Service{
 		orgID:   orgID,
@@ -107,7 +109,8 @@ func (t *Service) Apply(session *user.SessionState) error {
 	)
 
 	storage := t.storage
-	customPolicies, err := session.CustomPolicies()
+
+	customPolicies, err := session.GetCustomPolicies()
 	if err != nil {
 		policyIDs = session.PolicyIDs()
 	} else {
@@ -242,8 +245,9 @@ func (t *Service) Apply(session *user.SessionState) error {
 	return nil
 }
 
-func (t *Service) Logger() *logrus.Logger {
-	return t.logger
+// Logger implements a typical logger signature with service context.
+func (t *Service) Logger() *logrus.Entry {
+	return logrus.NewEntry(t.logger)
 }
 
 // ApplyRateLimits will write policy limits to session and apiLimits.
@@ -355,7 +359,7 @@ func (t *Service) applyPartitions(policy user.Policy, session *user.SessionState
 		if !usePartitions || policy.Partitions.Acl {
 			applyState.didAcl[k] = true
 
-			ar.AllowedURLs = copyAllowedURLs(v.AllowedURLs)
+			ar.AllowedURLs = MergeAllowedURLs(ar.AllowedURLs, v.AllowedURLs)
 
 			// Merge ACLs for the same API
 			if r, ok := rights[k]; ok {
@@ -365,19 +369,7 @@ func (t *Service) applyPartitions(policy user.Policy, session *user.SessionState
 				}
 				r.Versions = appendIfMissing(rights[k].Versions, v.Versions...)
 
-				for _, u := range v.AllowedURLs {
-					found := false
-					for ai, au := range r.AllowedURLs {
-						if u.URL == au.URL {
-							found = true
-							r.AllowedURLs[ai].Methods = appendIfMissing(au.Methods, u.Methods...)
-						}
-					}
-
-					if !found {
-						r.AllowedURLs = append(r.AllowedURLs, v.AllowedURLs...)
-					}
-				}
+				r.AllowedURLs = MergeAllowedURLs(r.AllowedURLs, v.AllowedURLs)
 
 				for _, t := range v.RestrictedTypes {
 					for ri, rt := range r.RestrictedTypes {

internal/policy/apply_test.go

@@ -9,11 +9,9 @@ import (
 	"testing"
 
 	"github.com/sirupsen/logrus"
-
-	"github.com/TykTechnologies/graphql-go-tools/pkg/graphql"
-
 	"github.com/stretchr/testify/assert"
 
+	"github.com/TykTechnologies/graphql-go-tools/pkg/graphql"
 	"github.com/TykTechnologies/tyk/internal/policy"
 	"github.com/TykTechnologies/tyk/user"
 )
@@ -22,9 +20,9 @@ import (
 var testDataFS embed.FS
 
 func TestApplyRateLimits_PolicyLimits(t *testing.T) {
-	svc := &policy.Service{}
-
 	t.Run("policy limits unset", func(t *testing.T) {
+		svc := &policy.Service{}
+
 		session := &user.SessionState{
 			Rate: 5,
 			Per:  10,
@@ -44,6 +42,8 @@ func TestApplyRateLimits_PolicyLimits(t *testing.T) {
 	})
 
 	t.Run("policy limits apply all", func(t *testing.T) {
+		svc := &policy.Service{}
+
 		session := &user.SessionState{
 			Rate: 5,
 			Per:  10,
@@ -69,6 +69,8 @@ func TestApplyRateLimits_PolicyLimits(t *testing.T) {
 	// changes are applied to api limits, but skipped on
 	// the session as the session has a higher allowance.
 	t.Run("policy limits apply per-api", func(t *testing.T) {
+		svc := &policy.Service{}
+
 		session := &user.SessionState{
 			Rate: 15,
 			Per:  10,
@@ -93,6 +95,8 @@ func TestApplyRateLimits_PolicyLimits(t *testing.T) {
 	// As the policy defined a lower rate than apiLimits,
 	// no changes to api limits are applied.
 	t.Run("policy limits skip", func(t *testing.T) {
+		svc := &policy.Service{}
+
 		session := &user.SessionState{
 			Rate: 5,
 			Per:  10,
@@ -117,29 +121,26 @@ func TestApplyRateLimits_PolicyLimits(t *testing.T) {
 func TestApplyRateLimits_FromCustomPolicies(t *testing.T) {
 	svc := &policy.Service{}
 
-	t.Run("Custom policies", func(t *testing.T) {
-		session := &user.SessionState{}
-		session.SetCustomPolicies([]user.Policy{
-			{
-				ID:           "pol1",
-				Partitions:   user.PolicyPartitions{RateLimit: true},
-				Rate:         8,
-				Per:          1,
-				AccessRights: map[string]user.AccessDefinition{"a": {}},
-			},
-			{
-				ID:           "pol2",
-				Partitions:   user.PolicyPartitions{RateLimit: true},
-				Rate:         10,
-				Per:          1,
-				AccessRights: map[string]user.AccessDefinition{"a": {}},
-			},
-		})
-
-		svc.Apply(session)
-
-		assert.Equal(t, 10, int(session.Rate))
+	session := &user.SessionState{}
+	session.SetCustomPolicies([]user.Policy{
+		{
+			ID:           "pol1",
+			Partitions:   user.PolicyPartitions{RateLimit: true},
+			Rate:         8,
+			Per:          1,
+			AccessRights: map[string]user.AccessDefinition{"a": {}},
+		},
+		{
+			ID:           "pol2",
+			Partitions:   user.PolicyPartitions{RateLimit: true},
+			Rate:         10,
+			Per:          1,
+			AccessRights: map[string]user.AccessDefinition{"a": {}},
+		},
 	})
+
+	assert.NoError(t, svc.Apply(session))
+	assert.Equal(t, 10, int(session.Rate))
 }
 
 func TestApplyEndpointLevelLimits(t *testing.T) {
@@ -190,7 +191,7 @@ func testPrepareApplyPolicies(tb testing.TB) (*policy.Service, []testApplyPolici
 	err = json.Unmarshal(f, &repoPols)
 	assert.NoError(tb, err)
 
-	store := policy.NewStore(repoPols)
+	store := policy.NewStoreMap(repoPols)
 	orgID := ""
 	service := policy.New(&orgID, store, logrus.StandardLogger())
 

internal/policy/store.go

@@ -4,32 +4,45 @@ import (
 	"github.com/TykTechnologies/tyk/user"
 )
 
-// Store is an in-memory policy storage object that
-// implements the repository for policy access. We
-// do not implement concurrency protections here.
+// Store is an in-memory policy storage object that implements the
+// repository for policy access. We  do not implement concurrency
+// protections here. Where order is important, use this.
 type Store struct {
-	policies map[string]user.Policy
+	policies []user.Policy
 }
 
-func NewStore(policies map[string]user.Policy) *Store {
+// NewStore returns a new policy.Store.
+func NewStore(policies []user.Policy) *Store {
 	return &Store{
 		policies: policies,
 	}
 }
 
+// PolicyIDs returns a list policy IDs in the store.
+// It will return nil if no policies exist.
 func (s *Store) PolicyIDs() []string {
+	if len(s.policies) == 0 {
+		return nil
+	}
+
 	policyIDs := make([]string, 0, len(s.policies))
 	for _, val := range s.policies {
 		policyIDs = append(policyIDs, val.ID)
 	}
 	return policyIDs
 }
 
+// PolicyByID returns a policy by ID.
 func (s *Store) PolicyByID(id string) (user.Policy, bool) {
-	v, ok := s.policies[id]
-	return v, ok
+	for _, pol := range s.policies {
+		if pol.ID == id {
+			return pol, true
+		}
+	}
+	return user.Policy{}, false
 }
 
+// PolicyCount returns the number of policies in the store.
 func (s *Store) PolicyCount() int {
 	return len(s.policies)
 }

internal/policy/store_map.go

@@ -0,0 +1,46 @@
+package policy
+
+import (
+	"github.com/TykTechnologies/tyk/user"
+)
+
+// StoreMap is same as Store, but doesn't preserve order.
+type StoreMap struct {
+	policies map[string]user.Policy
+}
+
+// NewStoreMap returns a new policy.StoreMap.
+func NewStoreMap(policies map[string]user.Policy) *StoreMap {
+	if len(policies) == 0 {
+		policies = make(map[string]user.Policy)
+	}
+
+	return &StoreMap{
+		policies: policies,
+	}
+}
+
+// PolicyIDs returns a list policy IDs in the store.
+// It will return nil if no policies exist.
+func (s *StoreMap) PolicyIDs() []string {
+	if len(s.policies) == 0 {
+		return nil
+	}
+
+	policyIDs := make([]string, 0, len(s.policies))
+	for _, val := range s.policies {
+		policyIDs = append(policyIDs, val.ID)
+	}
+	return policyIDs
+}
+
+// PolicyByID returns a policy by ID.
+func (s *StoreMap) PolicyByID(id string) (user.Policy, bool) {
+	v, ok := s.policies[id]
+	return v, ok
+}
+
+// PolicyCount returns the number of policies in the store.
+func (s *StoreMap) PolicyCount() int {
+	return len(s.policies)
+}