Merging to release-5.4.0: TT-12347 fix management of custom keys in mdcb installations (#6353)

[buger]

User description

TT-12347 fix management of custom keys in mdcb installations (#6353)

User description

Description

Related Issue

https://tyktech.atlassian.net/browse/TT-12347

Motivation and Context

How This Has Been Tested

Screenshots (if appropriate)

Types of changes

• 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

• 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

Enhancement

---

Description

• Changed the order of key search logic in the SessionDetail method of
  DefaultSessionManager.
• Moved keyName to be appended after generating token with orgID.

---

Changes walkthrough 📝

	Relevant files
Enhancement	auth_manager.go Modify key search order in `SessionDetail` method                gateway/auth_manager.go Changed the order of key search logic in SessionDetail method. Moved keyName to be appended after generating token with orgID. +3/-1

---

💡 PR-Agent usage:
Comment /help on the PR to get a list of all available PR-Agent tools
and their descriptions

---

PR Type

Enhancement, Tests

---

Description

• Updated the key search logic in the SessionDetail method to handle custom keys more effectively.
• Enhanced the ProcessKeySpaceChanges method to include fallback deletion using token ID.
• Added new tests to verify the processing of custom keys in the edge gateway.
• Included tests for fallback key deletion using token ID.

---

Changes walkthrough 📝

	Relevant files
Enhancement	auth_manager.go Update key search logic in SessionDetail method                    gateway/auth_manager.go Changed the order of key search logic in the SessionDetail method. Added custom key handling in the key search process. +2/-2      rpc_storage_handler.go Enhance key deletion logic in ProcessKeySpaceChanges          gateway/rpc_storage_handler.go Modified ProcessKeySpaceChanges to handle custom key deletion. Added deleteUsingTokenID function for fallback deletion using token ID. +18/-1
Tests	auth_manager_test.go Add tests for custom key processing in edge gateway            gateway/auth_manager_test.go Added TestCustomKeysEdgeGw to check custom key processing. Included test cases for custom keys and base64 representation. +98/-0    rpc_storage_handler_test.go Add tests for fallback key deletion using token ID              gateway/rpc_storage_handler_test.go Added TestDeleteUsingTokenID to verify fallback deletion using token ID. Included test cases for custom key ID and non-existent TokenID. +63/-0

---

💡 PR-Agent usage:
Comment /help on the PR to get a list of all available PR-Agent tools and their descriptions

[github-actions]

PR Reviewer Guide 🔍

⏱️ Estimated effort to review [1-5]	4
🧪 Relevant tests	Yes
🔒 Security concerns	No
⚡ Key issues to review	Possible Bug: The logic in gateway/auth_manager.go modifies the toSearchList by first clearing it and then adding keyName back in a separate step. This could potentially introduce bugs or inefficiencies in key management. Consider refactoring to append directly without clearing first.
	Redundancy: In gateway/rpc_storage_handler.go, the error handling and logging within deleteUsingTokenID might be verbose and could be streamlined for clarity and performance.

[github-actions]

API Changes

no api changes detected

[github-actions]

PR Code Suggestions ✨

Category	Suggestion	Score
Best practice	Prevent deletion operations on empty keys to avoid errors Add a check to ensure that the key is not empty before attempting deletion operations in deleteUsingTokenID to prevent unnecessary processing and potential errors. gateway/rpc_storage_handler.go [1156-1158] +if key == "" { + log.Warn("attempted to delete an empty key") + return http.StatusBadRequest, fmt.Errorf("key is empty") +} id, err := storage.TokenID(key) if err == nil { _, status = r.Gw.handleDeleteKey(id, orgId, "-1", resetQuota) } Suggestion importance[1-10]: 9 Why: This suggestion is highly relevant as it adds a safeguard against processing empty keys, which can prevent runtime errors and unnecessary operations, thereby improving the reliability of the code.	9
	Handle cases where Token ID is not found more explicitly in the deletion logic Ensure that the deleteUsingTokenID function handles the case where TokenID is not found more gracefully, possibly by returning a custom error or a specific status code. gateway/rpc_storage_handler.go [1156-1158] id, err := storage.TokenID(key) -if err == nil { - _, status = r.Gw.handleDeleteKey(id, orgId, "-1", resetQuota) +if err != nil { + log.Warnf("Token ID not found for key: %v", key) + return http.StatusNotFound, err } +_, status = r.Gw.handleDeleteKey(id, orgId, "-1", resetQuota) Suggestion importance[1-10]: 8 Why: This suggestion improves the robustness of the function by explicitly handling cases where the TokenID is not found, which can prevent potential issues and make the code more maintainable.	8
Enhancement	Improve error handling and logging in key deletion logic Refactor the error handling in deleteUsingTokenID to handle different error types distinctly and log them appropriately for better debugging and maintenance. gateway/rpc_storage_handler.go [1122] if err != nil { - log.Debugf("cannot remove key:%v status: %v", key, status) + if errors.Is(err, someSpecificError) { + log.Errorf("specific error occurred: %v", err) + } else { + log.Errorf("error removing key: %v, status: %v", key, status) + } } Suggestion importance[1-10]: 7 Why: The suggestion improves error handling by distinguishing between different error types and logging them appropriately, which enhances debugging and maintenance. However, it is not crucial for the functionality of the code.	7

[sonarcloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
92.3% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud