TykTechnologies / tyk

Tyk Open Source API Gateway written in Go, supporting REST, GraphQL, TCP and gRPC protocols
Other
9.76k stars 1.09k forks source link

Merging to release-5.3: [TT-12897] Merge path based permissions when combining policies (#6597) #6625

Closed buger closed 1 month ago

buger commented 1 month ago

TT-12897 Merge path based permissions when combining policies (#6597)

User description

TT-12897
Summary [Security]Path-Based Permissions permissions in policies are not preserved when policies are combined
Type Bug Bug
Status In Dev
Points N/A
Labels customer_bug, jira_escalated

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


PR Type

Bug fix, Enhancement, Tests


Description


Changes walkthrough 📝

Relevant files
Enhancement
apply.go
Enhance policy application logic and logging                         

internal/policy/apply.go
  • Introduced MergeAllowedURLs function to merge allowed URLs.
  • Updated Logger function to return a logrus.Entry.
  • Changed session.CustomPolicies() to session.GetCustomPolicies().
  • +9/-17   
    store.go
    Refactor Store to use slice for policies                                 

    internal/policy/store.go
  • Changed Store to use a slice for policies.
  • Updated methods to accommodate slice-based storage.
  • +20/-7   
    store_map.go
    Add StoreMap for unordered policy storage                               

    internal/policy/store_map.go
  • Introduced StoreMap for unordered policy storage.
  • Implemented methods for StoreMap.
  • +46/-0   
    util.go
    Introduce MergeAllowedURLs and remove unused functions     

    internal/policy/util.go
  • Added MergeAllowedURLs function for merging URL access specs.
  • Removed copyAllowedURLs and contains functions.
  • +46/-70 
    custom_policies.go
    Enhance custom policies handling with order preservation 

    user/custom_policies.go
  • Added GetCustomPolicies to preserve policy order.
  • Updated CustomPolicies to use GetCustomPolicies.
  • +21/-7   
    Tests
    apply_test.go
    Update tests for policy application                                           

    internal/policy/apply_test.go
  • Added initialization of policy.Service in tests.
  • Ensured Apply method is tested with assert.NoError.
  • +29/-28 
    util_test.go
    Add tests for MergeAllowedURLs function                                   

    internal/policy/util_test.go - Added tests for `MergeAllowedURLs` function.
    +64/-0   
    Configuration changes
    Taskfile.yml
    Update Taskfile with stress test task                                       

    internal/policy/Taskfile.yml
  • Added stress task for running stress tests.
  • Updated default task to include test.
  • +16/-0   

    💡 PR-Agent usage: Comment /help "your question" on any pull request to receive relevant information


    Co-authored-by: Tit Petric tit@tyk.io

    sonarcloud[bot] commented 1 month ago

    Quality Gate Failed Quality Gate failed

    Failed conditions
    0.0% Coverage on New Code (required ≥ 80%)
    4.2% Duplication on New Code (required ≤ 3%)

    See analysis details on SonarCloud