fix(auth): harden jwt-auth empty claims_to_verify and key-auth anonymous fallback credential stripping#13468
Open
shreemaan-abhishek wants to merge 3 commits into
Conversation
An explicitly empty `claims_to_verify` array ([]) is truthy in Lua, so the `if not claims` guard in verify_claims() did not fall back to the default claims (exp/nbf). As a result the validation loop iterated over nothing and expired tokens were accepted when the array was set to []. Treat an empty array the same as an unset value and fall back to the default exp/nbf checks. Add tests covering the default-claims path, the empty-array case, and tokens that legitimately omit exp.
When a request carried an invalid API key and key-auth fell back to the configured anonymous consumer, the credential was forwarded upstream even with hide_credentials enabled: find_consumer() only strips credentials on the successful-auth path. Strip the credential before falling back to the anonymous consumer when hide_credentials is true. A request may carry the key in both the header and the query string, so clean up both. Add tests covering the header, query, and both-present cases on anonymous fallback.
dosubot
Bot
added
size:L
The anonymous-fallback tests scanned the error log (no_error_log:
invalid-key) and used a non-existent --- response_args section. nginx
always logs the original client request line (".../echo?auth=invalid-key"),
so no_error_log matched that logged request line rather than a forwarded
credential, failing in CI even though the credential was correctly
stripped. response_args is not a Test::Nginx section, so the query-strip
path was effectively unasserted.
Add a print_request_received upstream that echoes the received request
URI and headers, and assert the proxied request carries the anonymous
marker but not the credential. This checks what actually reaches the
upstream and fails closed if stripping regresses.
membphis
approved these changes
Jun 5, 2026
AlinsRan
approved these changes
Jun 5, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
This PR contains two small hardening fixes in the auth plugins, each with tests.
1.
jwt-auth: treat an emptyclaims_to_verifyarray as unsetverify_claims()falls back to the default claims (exp/nbf) whenclaims_to_verifyis not configured. The guard usedif not claims then, but an explicitly empty array ("claims_to_verify": []) is truthy in Lua, so it skipped the fallback and the validation loop iterated over nothing — meaning an expired token was accepted whenclaims_to_verifywas set to[]. The schema allows an empty array (nominItems), so this configuration is reachable.The guard now treats an empty array the same as an unset value (
if not claims or #claims == 0 then), restoring the defaultexp/nbfchecks. Behaviour for tokens that legitimately omitexp/nbfis unchanged (they are validated only if present).2.
key-auth: strip credentials on anonymous-consumer fallbackWhen a request carried an invalid API key and key-auth fell back to the configured
anonymous_consumer, the (invalid) credential was still forwarded upstream even withhide_credentialsenabled:find_consumer()only strips credentials on the successful-auth path. The fix strips the credential before falling back to the anonymous consumer whenhide_credentialsis true. Since a request can carry the key in both the header and the query string, both are cleaned up.Which issue(s) this PR fixes:
Fixes #
Checklist