Skip to content

Commit 9416376

Browse files
Revert "Merging to release-5.2: [TT-10109] Fix policy lookup map dist… (#5755)
…ortion (#5730)" This reverts commit c6dc825. <!-- Provide a general summary of your changes in the Title above --> ## Description <!-- Describe your changes in detail --> ## Related Issue <!-- This project only accepts pull requests related to open issues. --> <!-- If suggesting a new feature or change, please discuss it in an issue first. --> <!-- If fixing a bug, there should be an issue describing it with steps to reproduce. --> <!-- OSS: Please link to the issue here. Tyk: please create/link the JIRA ticket. --> ## Motivation and Context <!-- Why is this change required? What problem does it solve? --> ## How This Has Been Tested <!-- Please describe in detail how you tested your changes --> <!-- Include details of your testing environment, and the tests --> <!-- you ran to see how your change affects other areas of the code, etc. --> <!-- This information is helpful for reviewers and QA. --> ## Screenshots (if appropriate) ## Types of changes <!-- What types of changes does your code introduce? Put an `x` in all the boxes that apply: --> - [ ] Bug fix (non-breaking change which fixes an issue) - [ ] New feature (non-breaking change which adds functionality) - [ ] Breaking change (fix or feature that would cause existing functionality to change) - [ ] Refactoring or add test (improvements in base code or adds test coverage to functionality) ## Checklist <!-- Go over all the following points, and put an `x` in all the boxes that apply --> <!-- If there are no documentation updates required, mark the item as checked. --> <!-- Raise up any additional concerns not covered by the checklist. --> - [ ] I ensured that the documentation is up to date - [ ] I explained why this PR updates go.mod in detail with reasoning why it's required - [ ] I would like a code coverage CI quality gate exception and have explained why
1 parent c6dc825 commit 9416376

File tree

3 files changed

+4
-92
lines changed

3 files changed

+4
-92
lines changed

gateway/middleware.go

Lines changed: 0 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -492,8 +492,6 @@ func (t BaseMiddleware) ApplyPolicies(session *user.SessionState) error {
492492
if !usePartitions || policy.Partitions.Acl {
493493
didACL[k] = true
494494

495-
ar.AllowedURLs = copyAllowedURLs(v.AllowedURLs)
496-
497495
// Merge ACLs for the same API
498496
if r, ok := rights[k]; ok {
499497
// If GQL introspection is disabled, keep that configuration.
@@ -762,26 +760,6 @@ func (t BaseMiddleware) ApplyPolicies(session *user.SessionState) error {
762760
return nil
763761
}
764762

765-
func copyAllowedURLs(input []user.AccessSpec) []user.AccessSpec {
766-
if input == nil {
767-
return nil
768-
}
769-
770-
copied := make([]user.AccessSpec, len(input))
771-
772-
for i, as := range input {
773-
copied[i] = user.AccessSpec{
774-
URL: as.URL,
775-
}
776-
if as.Methods != nil {
777-
copied[i].Methods = make([]string, len(as.Methods))
778-
copy(copied[i].Methods, as.Methods)
779-
}
780-
}
781-
782-
return copied
783-
}
784-
785763
// CheckSessionAndIdentityForValidKey will check first the Session store for a valid key, if not found, it will try
786764
// the Auth Handler, if not found it will fail
787765
func (t BaseMiddleware) CheckSessionAndIdentityForValidKey(originalKey string, r *http.Request) (user.SessionState, bool) {

gateway/middleware_test.go

Lines changed: 0 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -343,65 +343,3 @@ func TestSessionLimiter_RedisQuotaExceeded_PerAPI(t *testing.T) {
343343
sendReqAndCheckQuota(t, apis[2].APIID, 24, false)
344344
sendReqAndCheckQuota(t, apis[2].APIID, 23, false)
345345
}
346-
347-
func TestCopyAllowedURLs(t *testing.T) {
348-
testCases := []struct {
349-
name string
350-
input []user.AccessSpec
351-
}{
352-
{
353-
name: "Copy non-empty slice of AccessSpec with non-empty Methods",
354-
input: []user.AccessSpec{
355-
{
356-
URL: "http://example.com",
357-
Methods: []string{"GET", "POST"},
358-
},
359-
{
360-
URL: "http://example.org",
361-
Methods: []string{"GET"},
362-
},
363-
},
364-
},
365-
{
366-
name: "Copy non-empty slice of AccessSpec with empty Methods",
367-
input: []user.AccessSpec{
368-
{
369-
URL: "http://example.com",
370-
Methods: []string{},
371-
},
372-
{
373-
URL: "http://example.org",
374-
Methods: []string{},
375-
},
376-
},
377-
},
378-
{
379-
name: "Copy non-empty slice of AccessSpec with nil Methods",
380-
input: []user.AccessSpec{
381-
{
382-
URL: "http://example.com",
383-
Methods: nil,
384-
},
385-
{
386-
URL: "http://example.org",
387-
Methods: nil,
388-
},
389-
},
390-
},
391-
{
392-
name: "Copy empty slice of AccessSpec",
393-
input: []user.AccessSpec{},
394-
},
395-
{
396-
name: "Copy nil slice of AccessSpec",
397-
input: []user.AccessSpec(nil),
398-
},
399-
}
400-
401-
for _, tc := range testCases {
402-
t.Run(tc.name, func(t *testing.T) {
403-
copied := copyAllowedURLs(tc.input)
404-
assert.Equal(t, tc.input, copied)
405-
})
406-
}
407-
}

gateway/policy_test.go

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -304,7 +304,7 @@ func (s *Test) TestPrepareApplyPolicies() (*BaseMiddleware, []testApplyPoliciesD
304304
ID: "per_path_1",
305305
AccessRights: map[string]user.AccessDefinition{"a": {
306306
AllowedURLs: []user.AccessSpec{
307-
{URL: "/user", Methods: []string{"GET", "POST"}},
307+
{URL: "/user", Methods: []string{"GET"}},
308308
},
309309
}, "b": {
310310
AllowedURLs: []user.AccessSpec{
@@ -316,7 +316,7 @@ func (s *Test) TestPrepareApplyPolicies() (*BaseMiddleware, []testApplyPoliciesD
316316
ID: "per_path_2",
317317
AccessRights: map[string]user.AccessDefinition{"a": {
318318
AllowedURLs: []user.AccessSpec{
319-
{URL: "/user", Methods: []string{"GET"}},
319+
{URL: "/user", Methods: []string{"GET", "POST"}},
320320
{URL: "/companies", Methods: []string{"GET", "POST"}},
321321
},
322322
}},
@@ -752,7 +752,7 @@ func (s *Test) TestPrepareApplyPolicies() (*BaseMiddleware, []testApplyPoliciesD
752752
{
753753
name: "Merge per path rules for the same API",
754754
policies: []string{"per-path2", "per-path1"},
755-
sessMatch: func(t *testing.T, sess *user.SessionState) {
755+
sessMatch: func(t *testing.T, s *user.SessionState) {
756756
want := map[string]user.AccessDefinition{
757757
"a": {
758758
AllowedURLs: []user.AccessSpec{
@@ -769,11 +769,7 @@ func (s *Test) TestPrepareApplyPolicies() (*BaseMiddleware, []testApplyPoliciesD
769769
},
770770
}
771771

772-
assert.Equal(t, user.AccessSpec{
773-
URL: "/user", Methods: []string{"GET"},
774-
}, s.Gw.getPolicy("per-path2").AccessRights["a"].AllowedURLs[0])
775-
776-
assert.Equal(t, want, sess.AccessRights)
772+
assert.Equal(t, want, s.AccessRights)
777773
},
778774
},
779775
{

0 commit comments

Comments
 (0)