Skip to content

Commit

Permalink
Fix allowance scope of the first policy if multiple policies are used (…
Browse files Browse the repository at this point in the history 
…TykTechnologies#2554)

Fix TykTechnologies#2550
  • Loading branch information
@buger
buger authored Sep 25, 2019
1 parent 8f80fbc commit 17becf4
Show file tree
Hide file tree
Showing 3 changed files with 82 additions and 45 deletions.
40 changes: 20 additions & 20 deletions gateway/middleware.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -342,6 +342,7 @@ func (t BaseMiddleware) ApplyPolicies(session *user.SessionState) error {
}

accessRights.AllowanceScope = apiID
accessRights.Limit.SetBy = apiID

// overwrite session access right for this API
rights[apiID] = accessRights
Expand All @@ -352,17 +353,6 @@ func (t BaseMiddleware) ApplyPolicies(session *user.SessionState) error {
didRateLimit[apiID] = true
}
} else {
multiAclPolicies := false
if i > 0 {
// Check if policy works with new APIs
for pa := range policy.AccessRights {
if _, ok := rights[pa]; !ok {
multiAclPolicies = true
break
}
}
}

usePartitions := policy.Partitions.Quota || policy.Partitions.RateLimit || policy.Partitions.Acl

for k, v := range policy.AccessRights {
Expand Down Expand Up @@ -396,6 +386,8 @@ func (t BaseMiddleware) ApplyPolicies(session *user.SessionState) error {

ar = &r
}

ar.Limit.SetBy = policy.ID
}

if !usePartitions || policy.Partitions.Quota {
Expand Down Expand Up @@ -431,14 +423,6 @@ func (t BaseMiddleware) ApplyPolicies(session *user.SessionState) error {
}
}

if multiAclPolicies && (!usePartitions || (policy.Partitions.Quota || policy.Partitions.RateLimit)) {
ar.AllowanceScope = policy.ID
}

if !multiAclPolicies {
ar.Limit.QuotaRenews = session.QuotaRenews
}

// Respect existing QuotaRenews
if r, ok := session.AccessRights[k]; ok && r.Limit != nil {
ar.Limit.QuotaRenews = r.Limit.QuotaRenews
Expand Down Expand Up @@ -484,10 +468,17 @@ func (t BaseMiddleware) ApplyPolicies(session *user.SessionState) error {

// set tags
session.Tags = []string{}
for tag, _ := range tags {
for tag := range tags {
session.Tags = append(session.Tags, tag)
}

distinctACL := map[string]bool{}
for _, v := range rights {
if v.Limit.SetBy != "" {
distinctACL[v.Limit.SetBy] = true
}
}

// If some APIs had only ACL partitions, inherit rest from session level
for k, v := range rights {
if !didRateLimit[k] {
Expand All @@ -502,6 +493,15 @@ func (t BaseMiddleware) ApplyPolicies(session *user.SessionState) error {
v.Limit.QuotaRenewalRate = session.QuotaRenewalRate
v.Limit.QuotaRenews = session.QuotaRenews
}

// If multime ACL
if len(distinctACL) > 1 {
if v.AllowanceScope == "" && v.Limit.SetBy != "" {
v.AllowanceScope = v.Limit.SetBy
}
}

rights[k] = v
}

// If we have policies defining rules for one single API, update session root vars (legacy)
Expand Down
85 changes: 61 additions & 24 deletions gateway/policy_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,11 +62,27 @@ type testApplyPoliciesData struct {
}

func testPrepareApplyPolicies() (*BaseMiddleware, []testApplyPoliciesData) {
assert := func(t *testing.T, want, expect interface{}) {
if jsonMarshalString(want) != jsonMarshalString(expect) {
t.Fatalf("want %v got %v", jsonMarshalString(want), jsonMarshalString(expect))
}
}

policiesMu.RLock()
policiesByID = map[string]user.Policy{
"nonpart1": {},
"nonpart2": {},
"difforg": {OrgID: "different"},
"nonpart1": {
ID: "p1",
AccessRights: map[string]user.AccessDefinition{"a": {}},
},
"nonpart2": {
ID: "p2",
AccessRights: map[string]user.AccessDefinition{"b": {}},
},
"nonpart3": {
ID: "p3",
AccessRights: map[string]user.AccessDefinition{"a": {}, "b": {}},
},
"difforg": {OrgID: "different"},
"tags1": {
Partitions: user.PolicyPartitions{Quota: true},
Tags: []string{"tagA"},
Expand Down Expand Up @@ -236,8 +252,38 @@ func testPrepareApplyPolicies() (*BaseMiddleware, []testApplyPoliciesData) {
"different org", nil, nil,
},
{
"MultiNonPart", []string{"nonpart1", "nonpart2"},
"", nil, nil,
name: "MultiNonPart",
policies: []string{"nonpart1", "nonpart2"},
sessMatch: func(t *testing.T, s *user.SessionState) {
want := map[string]user.AccessDefinition{
"a": {
Limit: &user.APILimit{},
AllowanceScope: "p1",
},
"b": {
Limit: &user.APILimit{},
AllowanceScope: "p2",
},
}

assert(t, want, s.AccessRights)
},
},
{
name: "MultiACLPolicy",
policies: []string{"nonpart3"},
sessMatch: func(t *testing.T, s *user.SessionState) {
want := map[string]user.AccessDefinition{
"a": {
Limit: &user.APILimit{},
},
"b": {
Limit: &user.APILimit{},
},
}

assert(t, want, s.AccessRights)
},
},
{
"NonpartAndPart", []string{"nonpart1", "quota1"},
Expand All @@ -248,9 +294,8 @@ func testPrepareApplyPolicies() (*BaseMiddleware, []testApplyPoliciesData) {
"", func(t *testing.T, s *user.SessionState) {
want := []string{"key-tag", "tagA", "tagX", "tagY"}
sort.Strings(s.Tags)
if !reflect.DeepEqual(want, s.Tags) {
t.Fatalf("want Tags %v, got %v", want, s.Tags)
}

assert(t, want, s.Tags)
}, &user.SessionState{
Tags: []string{"key-tag"},
},
Expand Down Expand Up @@ -307,18 +352,15 @@ func testPrepareApplyPolicies() (*BaseMiddleware, []testApplyPoliciesData) {
"AclPart", []string{"acl1"},
"", func(t *testing.T, s *user.SessionState) {
want := map[string]user.AccessDefinition{"a": {Limit: &user.APILimit{}}}
if !reflect.DeepEqual(want, s.AccessRights) {
t.Fatalf("want %v got %v", want, s.AccessRights)
}

assert(t, want, s.AccessRights)
}, nil,
},
{
"AclPart", []string{"acl1", "acl2"},
"", func(t *testing.T, s *user.SessionState) {
want := map[string]user.AccessDefinition{"a": {Limit: &user.APILimit{}}, "b": {Limit: &user.APILimit{}}}
if !reflect.DeepEqual(want, s.AccessRights) {
t.Fatalf("want %v got %v", want, s.AccessRights)
}
assert(t, want, s.AccessRights)
}, nil,
},
{
Expand All @@ -334,10 +376,7 @@ func testPrepareApplyPolicies() (*BaseMiddleware, []testApplyPoliciesData) {
if err != nil {
t.Fatalf("couldn't apply policy: %s", err.Error())
}
want := newPolicy.AccessRights
if !reflect.DeepEqual(want, s.AccessRights) {
t.Fatalf("want %v got %v", want, s.AccessRights)
}
assert(t, newPolicy.AccessRights, s.AccessRights)
}, nil,
},
{
Expand Down Expand Up @@ -373,9 +412,8 @@ func testPrepareApplyPolicies() (*BaseMiddleware, []testApplyPoliciesData) {
AllowanceScope: "c",
},
}
if !reflect.DeepEqual(want, s.AccessRights) {
t.Fatalf("want %v got %v", want, s.AccessRights)
}

assert(t, want, s.AccessRights)
},
},
{
Expand Down Expand Up @@ -416,9 +454,8 @@ func testPrepareApplyPolicies() (*BaseMiddleware, []testApplyPoliciesData) {
AllowanceScope: "d",
},
}
if !reflect.DeepEqual(want, s.AccessRights) {
t.Fatalf("want %v got %v", want, s.AccessRights)
}

assert(t, want, s.AccessRights)
},
},
}
Expand Down
2 changes: 1 addition & 1 deletion user/session.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ type APILimit struct {
QuotaRenews int64 `json:"quota_renews" msg:"quota_renews"`
QuotaRemaining int64 `json:"quota_remaining" msg:"quota_remaining"`
QuotaRenewalRate int64 `json:"quota_renewal_rate" msg:"quota_renewal_rate"`
SetByPolicy bool `json:"set_by_policy" msg:"set_by_policy"`
SetBy string `json:"-" msg:"-"`
}

// AccessDefinition defines which versions of an API a key has access to
Expand Down

0 comments on commit 17becf4

Please sign in to comment.