Merging to release-5.3: [TT-12741] Looped ap is wrongfully inherit the caller's authentication key when using url rewrite (#6778)

[buger]

User description

TT-12741 Looped ap is wrongfully inherit the caller's authentication key when using url rewrite (#6778)

User description

TT-12741

Summary	Looped APIs wrongfully inherit the caller's Authentication key when using URL rewrite
Type	Bug
Status	In Dev
Points	N/A
Labels	'24Bugsmash, customer_bug, jira_escalated

---

PR to see CI/CD result, please don't merge it.

---

PR Type

Bug fix, Tests

---

Description

• Introduced a new context constant SelfLooping and methods
  ctxSetSelfLooping and ctxSelfLooping to manage self-looping state in
  requests.
• Updated ctxCheckLimits to bypass rate limits and quotas for
  self-looping requests.
• Modified API loader to set self-looping state for self-referencing
  requests.
• Enhanced the test TestQuotaNotAppliedWithURLRewrite to include
  scenarios for self-looping and URL rewrite, ensuring proper behavior.

---

Changes walkthrough 📝

	Relevant files
Enhancement	ctx.go Add support for managing self-looping state in context      ctx/ctx.go Added a new constant SelfLooping to the context. Introduced new methods ctxSetSelfLooping and ctxSelfLooping for managing self-looping state in requests. +1/-0
Bug fix	api.go Update rate limit and quota checks for self-looping requests gateway/api.go Modified ctxCheckLimits to skip rate limits and quotas for self-looping requests. Added logic to check and set self-looping state in requests. +20/-1    api_loader.go Set self-looping state for self-referencing requests          gateway/api_loader.go Added logic to set self-looping state when the hostname is "self". +1/-0
Tests	middleware_test.go Enhance tests to cover self-looping and URL rewrite scenarios gateway/middleware_test.go Updated TestQuotaNotAppliedWithURLRewrite to include extended paths and self-looping scenarios. Added a loader to create a merged API spec for testing. +7/-1

---

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

---

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

---

PR Type

Bug fix, Tests

---

Description

• Introduced a new context constant SelfLooping and methods ctxSetSelfLooping and ctxSelfLooping to manage self-looping state in requests.
• Updated ctxCheckLimits to bypass rate limits and quotas for self-looping requests.
• Modified API loader to set self-looping state for self-referencing requests.
• Enhanced tests to validate self-looping behavior, including scenarios with authentication tokens and URL rewrites.
• Added utilities and unit tests for managing and checking self-looping state in requests.

---

Changes walkthrough 📝

	Relevant files
Enhancement	ctx.go Add support for managing self-looping state in context      ctx/ctx.go Added a new constant SelfLooping to the context. Introduced methods ctxSetSelfLooping and ctxSelfLooping for managing self-looping state in requests. +1/-0      looping.go Add utilities for managing self-looping state in requests internal/httpctx/looping.go Introduced SetSelfLooping and IsSelfLooping methods to manage and check self-looping state in requests. +19/-0
Bug fix	api.go Update rate limit and quota checks for self-looping requests gateway/api.go Updated ctxCheckLimits to bypass rate limits and quotas for self-looping requests. Integrated logic to check and set self-looping state in requests. +7/-0      api_loader.go Set self-looping state for self-referencing requests          gateway/api_loader.go Added logic to set self-looping state when the hostname is "self". +2/-0      mw_auth_key.go Skip auth key checks for self-looping requests                      gateway/mw_auth_key.go Updated logic to skip authentication key checks for self-looping requests. +6/-7
Tests	looping_test.go Add test for self-looping with authentication tokens          gateway/looping_test.go Added a new test TestLooping_AnotherAPIWithAuthTokens to validate self-looping behavior with authentication tokens. +95/-0    middleware_test.go Enhance tests to cover self-looping and URL rewrite scenarios gateway/middleware_test.go Enhanced TestQuotaNotAppliedWithURLRewrite to include extended paths and self-looping scenarios. +2/-1      looping_test.go Add tests for self-looping state management utilities        internal/httpctx/looping_test.go Added unit tests for SetSelfLooping and IsSelfLooping methods. +19/-0

---

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

[github-actions]

API Changes

--- prev.txt	2024-12-19 13:19:49.821970080 +0000
+++ current.txt	2024-12-19 13:19:46.900982308 +0000
@@ -6871,6 +6871,7 @@
 	// CacheOptions holds cache options required for cache writer middleware.
 	CacheOptions
 	OASDefinition
+	SelfLooping
 )
 # Package: ./dlpython
 

[github-actions]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis ✅ 6778 - Fully compliant Fully compliant requirements: Introduce a mechanism to identify and manage self-looping requests. Ensure rate limits and quotas are bypassed for self-looping requests. Update API loader to set self-looping state for self-referencing requests. Enhance tests to cover self-looping and URL rewrite scenarios. Not compliant requirements:

⏱️ Estimated effort to review: 4 🔵🔵🔵🔵⚪

🧪 PR contains tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Code Smell The function ctxCheckLimits now includes a conditional check for self-looping requests. Consider adding comments or documentation to clarify the purpose and implications of bypassing rate limits and quotas for self-looping requests. Code Smell The ProcessRequest method in AuthKey skips authentication checks for self-looping requests. Ensure this behavior is intentional and does not introduce unintended security or logical issues.

[github-actions]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Score
Possible issue	Add a nil check for the request context to prevent runtime errors Ensure that httpctx.IsSelfLooping(r) correctly handles nil or invalid request contexts to avoid potential runtime panics. gateway/api.go [3224-3226] -if httpctx.IsSelfLooping(r) { +if r == nil || httpctx.IsSelfLooping(r) { return false } Suggestion importance[1-10]: 8 Why: Adding a nil check for the request context is a critical improvement to prevent potential runtime panics when the request object is nil. This ensures robustness and stability in the code.	8
	Add a nil check for the request to avoid runtime errors in session and self-looping checks Ensure ctxGetSession(r) and httpctx.IsSelfLooping(r) handle nil or invalid requests to prevent runtime errors. gateway/mw_auth_key.go [97-98] -if ses := ctxGetSession(r); ses != nil && httpctx.IsSelfLooping(r) { +if r != nil && ses := ctxGetSession(r); ses != nil && httpctx.IsSelfLooping(r) { return nil, http.StatusOK } Suggestion importance[1-10]: 8 Why: Including a nil check for the request object ensures that the session and self-looping checks do not cause runtime errors. This is a critical improvement for maintaining code stability.	8
General	Add a check to ensure r.URL is valid before setting the self-looping flag Validate that r.URL.Hostname() is not empty or malformed before calling httpctx.SetSelfLooping to prevent unexpected behavior. gateway/api_loader.go [593-594] -if r.URL.Hostname() == "self" { +if r.URL != nil && r.URL.Hostname() == "self" { httpctx.SetSelfLooping(r, true) } Suggestion importance[1-10]: 7 Why: Validating r.URL before accessing its Hostname method is a good practice to avoid unexpected behavior or runtime errors. This suggestion enhances the code's reliability.	7
	Add error handling for nil requests in the SetSelfLooping function Add error handling or logging in SetSelfLooping to ensure that invalid or nil requests are not processed silently. internal/httpctx/looping.go [13] -selfLoopingValue.Set(r, value) +if r != nil { + selfLoopingValue.Set(r, value) +} else { + // Log or handle error for nil request +} Suggestion importance[1-10]: 6 Why: Adding error handling for nil requests in SetSelfLooping improves the function's robustness and ensures that invalid inputs are not processed silently. However, the suggestion could be more impactful if it included specific logging or error reporting.	6

[sonarqubecloud]

Quality Gate failed

Failed conditions
C Reliability Rating on New Code (required ≥ A)

See analysis details on SonarQube Cloud

Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE