Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
[TT-12897] Merge path based permissions when combining policies (#6597)
<details open> <summary><a href="https://tyktech.atlassian.net/browse/TT-12897" title="TT-12897" target="_blank">TT-12897</a></summary> <br /> <table> <tr> <th>Summary</th> <td>[Security]Path-Based Permissions permissions in policies are not preserved when policies are combined</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%20customer_bug%20ORDER%20BY%20created%20DESC" title="customer_bug">customer_bug</a>, <a href="https://tyktech.atlassian.net/issues?jql=project%20%3D%20TT%20AND%20labels%20%3D%20jira_escalated%20ORDER%20BY%20created%20DESC" title="jira_escalated">jira_escalated</a></td> </tr> </table> </details> <!-- do not remove this marker as it will break jira-lint's functionality. added_by_jira_lint --> --- PR uses custom policies to combine several policies with access rights set. Since a `map` was in the path, user API for custom policies needed an extension to preserve policy ID order. The existing function returning a map didn't handle json decode errors properly and go semantics when looping over maps don't preserve this order, but it's random so tests would fail. Verified with `task stress`. Issue: https://tyktech.atlassian.net/browse/TT-12897 ___ Bug fix, Enhancement, Tests ___ - Enhanced policy application logic by introducing `MergeAllowedURLs` to merge allowed URLs efficiently. - Refactored `Store` to use a slice for policies, and introduced `StoreMap` for unordered policy storage. - Improved custom policy handling by adding `GetCustomPolicies` to preserve policy order. - Updated tests to ensure proper application of policies and added new tests for `MergeAllowedURLs`. - Updated Taskfile to include a new `stress` task for running stress tests. ___ <table><thead><tr><th></th><th align="left">Relevant files</th></tr></thead><tbody><tr><td><strong>Enhancement</strong></td><td><table> <tr> <td> <details> <summary><strong>apply.go</strong><dd><code>Enhance policy application logic and logging</code> </dd></summary> <hr> internal/policy/apply.go <li>Introduced <code>MergeAllowedURLs</code> function to merge allowed URLs.<br> <li> Updated <code>Logger</code> function to return a <code>logrus.Entry</code>.<br> <li> Changed <code>session.CustomPolicies()</code> to <code>session.GetCustomPolicies()</code>. </details> </td> <td><a href="https://github.com/TykTechnologies/tyk/pull/6597/files#diff-59b92e9d31f142f1d99b746eb3ff7db4e26bf6c3044c9b87b58034a947ee04d1">+9/-17</a> </td> </tr> <tr> <td> <details> <summary><strong>store.go</strong><dd><code>Refactor Store to use slice for policies</code> </dd></summary> <hr> internal/policy/store.go <li>Changed <code>Store</code> to use a slice for policies.<br> <li> Updated methods to accommodate slice-based storage. </details> </td> <td><a href="https://github.com/TykTechnologies/tyk/pull/6597/files#diff-13dec7bc453c9ff99550c83d2f86a017bbf7fb863584dc30603af15d29ef9d3d">+20/-7</a> </td> </tr> <tr> <td> <details> <summary><strong>store_map.go</strong><dd><code>Add StoreMap for unordered policy storage</code> </dd></summary> <hr> internal/policy/store_map.go <li>Introduced <code>StoreMap</code> for unordered policy storage.<br> <li> Implemented methods for <code>StoreMap</code>. </details> </td> <td><a href="https://github.com/TykTechnologies/tyk/pull/6597/files#diff-24a7a95a1cf4f14b59a3475127dc45541357638d6949323255faeeb2ed657d27">+46/-0</a> </td> </tr> <tr> <td> <details> <summary><strong>util.go</strong><dd><code>Introduce MergeAllowedURLs and remove unused functions</code> </dd></summary> <hr> internal/policy/util.go <li>Added <code>MergeAllowedURLs</code> function for merging URL access specs.<br> <li> Removed <code>copyAllowedURLs</code> and <code>contains</code> functions. </details> </td> <td><a href="https://github.com/TykTechnologies/tyk/pull/6597/files#diff-0323c3da13f08a9ccd340ac04208d680856354fd566dffcad925fa6645639955">+46/-70</a> </td> </tr> <tr> <td> <details> <summary><strong>custom_policies.go</strong><dd><code>Enhance custom policies handling with order preservation</code> </dd></summary> <hr> user/custom_policies.go <li>Added <code>GetCustomPolicies</code> to preserve policy order.<br> <li> Updated <code>CustomPolicies</code> to use <code>GetCustomPolicies</code>. </details> </td> <td><a href="https://github.com/TykTechnologies/tyk/pull/6597/files#diff-911674993eef6c43a04edc0e90ea1f2e6d595792eef840d23b2e3deb1c8265c5">+21/-7</a> </td> </tr> </table></td></tr><tr><td><strong>Tests</strong></td><td><table> <tr> <td> <details> <summary><strong>apply_test.go</strong><dd><code>Update tests for policy application</code> </dd></summary> <hr> internal/policy/apply_test.go <li>Added initialization of <code>policy.Service</code> in tests.<br> <li> Ensured <code>Apply</code> method is tested with <code>assert.NoError</code>. </details> </td> <td><a href="https://github.com/TykTechnologies/tyk/pull/6597/files#diff-5af7e299a6b0ce11e22f8aa4a01854b1151f4b54dccc68f0cd1cbedee5aed7c8">+29/-28</a> </td> </tr> <tr> <td> <details> <summary><strong>util_test.go</strong><dd><code>Add tests for MergeAllowedURLs function</code> </dd></summary> <hr> internal/policy/util_test.go - Added tests for `MergeAllowedURLs` function. </details> </td> <td><a href="https://github.com/TykTechnologies/tyk/pull/6597/files#diff-c750a1b8a01d19dacf02ba7512b8e2b987bf8147cf3345a4374504d9d5b3840e">+64/-0</a> </td> </tr> </table></td></tr><tr><td><strong>Configuration changes</strong></td><td><table> <tr> <td> <details> <summary><strong>Taskfile.yml</strong><dd><code>Update Taskfile with stress test task</code> </dd></summary> <hr> internal/policy/Taskfile.yml <li>Added <code>stress</code> task for running stress tests.<br> <li> Updated <code>default</code> task to include <code>test</code>. </details> </td> <td><a href="https://github.com/TykTechnologies/tyk/pull/6597/files#diff-e0f19d4dd27acb397e19ccb080f3142a09f5978699da5843bfc71e7ffa4bb775">+16/-0</a> </td> </tr> </table></td></tr></tr></tbody></table> ___ > 💡 **PR-Agent usage**: Comment `/help "your question"` on any pull request to receive relevant information --------- Co-authored-by: Tit Petric <tit@tyk.io> (cherry picked from commit e31a08f)
- Loading branch information