Skip to content

Commit 0fce23c

Browse files
andrei-tykTit Petricbuger
authored
[TT-14365]Test/apply policies restricted types fix (#7020)
### **User description** <details open> <summary><a href="https://tyktech.atlassian.net/browse/TT-14365" title="TT-14365" target="_blank">TT-14365</a></summary> <br /> <table> <tr> <th>Summary</th> <td>Issue merging field based permissions, allowed_types should be merged</td> </tr> <tr> <th>Type</th> <td> <img alt="Bug" src="https://tyktech.atlassian.net/rest/api/2/universal_avatar/view/type/issuetype/avatar/10303?size=medium" /> Bug </td> </tr> <tr> <th>Status</th> <td>In Dev</td> </tr> <tr> <th>Points</th> <td>N/A</td> </tr> <tr> <th>Labels</th> <td><a href="https://tyktech.atlassian.net/issues?jql=project%20%3D%20TT%20AND%20labels%20%3D%20Commercial_candidate_rel2-2025%20ORDER%20BY%20created%20DESC" title="Commercial_candidate_rel2-2025">Commercial_candidate_rel2-2025</a>, <a href="https://tyktech.atlassian.net/issues?jql=project%20%3D%20TT%20AND%20labels%20%3D%20QA_Fail%20ORDER%20BY%20created%20DESC" title="QA_Fail">QA_Fail</a></td> </tr> </table> </details> <!-- do not remove this marker as it will break jira-lint's functionality. added_by_jira_lint --> --- <!-- 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 ___ ### **PR Type** Bug fix ___ ### **Description** - Map existing restricted types to indices - Intersect fields for common restricted types - Append non-existing restricted types from new policy ___ ### **Changes walkthrough** 📝 <table><thead><tr><th></th><th align="left">Relevant files</th></tr></thead><tbody><tr><td><strong>Bug fix</strong></td><td><table> <tr> <td> <details> <summary><strong>apply.go</strong><dd><code>Update merging logic for restricted types in policies</code>&nbsp; &nbsp; &nbsp; &nbsp; </dd></summary> <hr> internal/policy/apply.go <li>Introduced a map to track indices of restricted types<br> <li> Updated field intersection when types exist in both policies<br> <li> Append new restricted types when missing </details> </td> <td><a href="https://github.com/TykTechnologies/tyk/pull/7020/files#diff-59b92e9d31f142f1d99b746eb3ff7db4e26bf6c3044c9b87b58034a947ee04d1">+11/-4</a>&nbsp; &nbsp; </td> </tr> </table></td></tr></tr></tbody></table> ___ > <details> <summary> Need help?</summary><li>Type <code>/help how to ...</code> in the comments thread for any questions about PR-Agent usage.</li><li>Check out the <a href="https://qodo-merge-docs.qodo.ai/usage-guide/">documentation</a> for more information.</li></details> --------- Co-authored-by: Tit Petric <tit@tyk.io> Co-authored-by: Leonid Bugaev <leonsbox@gmail.com>
1 parent 7a2934e commit 0fce23c

File tree

2 files changed

+29
-5
lines changed

2 files changed

+29
-5
lines changed

internal/policy/apply.go

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -382,12 +382,24 @@ func (t *Service) applyPartitions(policy user.Policy, session *user.SessionState
382382
if len(r.RestrictedTypes) == 0 {
383383
r.RestrictedTypes = v.RestrictedTypes
384384
} else {
385+
// Create a map to track which types have been processed
386+
processedTypes := make(map[string]bool)
387+
385388
for _, t := range v.RestrictedTypes {
389+
typeFound := false
386390
for ri, rt := range r.RestrictedTypes {
387391
if t.Name == rt.Name {
388-
r.RestrictedTypes[ri].Fields = intersection(rt.Fields, t.Fields)
392+
// Merge fields for existing types
393+
r.RestrictedTypes[ri].Fields = appendIfMissing(rt.Fields, t.Fields...)
394+
typeFound = true
395+
processedTypes[t.Name] = true
396+
break
389397
}
390398
}
399+
// Add new types that don't exist in destination
400+
if !typeFound {
401+
r.RestrictedTypes = append(r.RestrictedTypes, t)
402+
}
391403
}
392404
}
393405

@@ -397,12 +409,24 @@ func (t *Service) applyPartitions(policy user.Policy, session *user.SessionState
397409
if len(r.AllowedTypes) == 0 {
398410
r.AllowedTypes = v.AllowedTypes
399411
} else {
412+
// Create a map to track which types have been processed
413+
processedTypes := make(map[string]bool)
414+
400415
for _, t := range v.AllowedTypes {
416+
typeFound := false
401417
for ri, rt := range r.AllowedTypes {
402418
if t.Name == rt.Name {
419+
// Merge fields for existing types
403420
r.AllowedTypes[ri].Fields = appendIfMissing(rt.Fields, t.Fields...)
421+
typeFound = true
422+
processedTypes[t.Name] = true
423+
break
404424
}
405425
}
426+
// Add new types that don't exist in destination
427+
if !typeFound {
428+
r.AllowedTypes = append(r.AllowedTypes, t)
429+
}
406430
}
407431
}
408432

internal/policy/apply_test.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -761,8 +761,8 @@ func testPrepareApplyPolicies(tb testing.TB) (*policy.Service, []testApplyPolici
761761
want := map[string]user.AccessDefinition{
762762
"a": {
763763
RestrictedTypes: []graphql.Type{
764-
{Name: "Country", Fields: []string{"code"}},
765-
{Name: "Person", Fields: []string{"name"}},
764+
{Name: "Country", Fields: []string{"code", "name", "phone"}},
765+
{Name: "Person", Fields: []string{"name", "height", "mass"}},
766766
},
767767
Limit: user.APILimit{},
768768
},
@@ -784,8 +784,8 @@ func testPrepareApplyPolicies(tb testing.TB) (*policy.Service, []testApplyPolici
784784
{Name: "Person", Fields: []string{"name", "height", "mass"}},
785785
},
786786
RestrictedTypes: []graphql.Type{
787-
{Name: "Dog", Fields: []string{"name", "breed"}},
788-
{Name: "Cat", Fields: []string{"name"}},
787+
{Name: "Dog", Fields: []string{"name", "breed", "country"}},
788+
{Name: "Cat", Fields: []string{"name", "country"}},
789789
},
790790
Limit: user.APILimit{},
791791
},

0 commit comments

Comments
 (0)