-
Notifications
You must be signed in to change notification settings - Fork 460
fix: add validation for duplicated API keys #5955
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #5955 +/- ##
==========================================
+ Coverage 65.80% 65.89% +0.08%
==========================================
Files 217 217
Lines 36020 36027 +7
==========================================
+ Hits 23703 23740 +37
+ Misses 10836 10808 -28
+ Partials 1481 1479 -2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
* reject duplicated API keys * enhance api-key-auth e2e test to cover duplicated client IDs Signed-off-by: Gavin Lam <[email protected]>
apiVersion: v1 | ||
kind: Secret | ||
metadata: | ||
namespace: gateway-conformance-infra | ||
name: api-key-auth-users-secret-2 | ||
data: | ||
# key2 - duplicate client id should be ignored | ||
client1: "a2V5Mg==" | ||
--- |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Enhanced APIKeyAuth e2e test to cover duplicate client IDs. Only the first API key should be retained. For instance, in {client1: "key1"}
+ {client1: "key2"}
, key2 is dropped.
This behavior is documented in the APIKeyAuth documentation.
* reject duplicated API keys * enhance api-key-auth e2e test to cover duplicated client IDs Signed-off-by: Gavin Lam <[email protected]> Signed-off-by: melsal13 <[email protected]>
What type of PR is this?
Bug fix
What this PR does / why we need it:
Duplicate API keys in apiKeyAuth can cause the entire controller to stall.
The proposed changes address this by rejecting the affected SecurityPolicy and reporting the error through a status message.
Which issue(s) this PR fixes:
Fixes #5729
Release Notes: Yes