Copilot commented on code in PR #13468:
URL: https://github.com/apache/apisix/pull/13468#discussion_r3377443024
##########
t/lib/server.lua:
##########
@@ -413,6 +413,22 @@ function _M.print_uri_detailed()
ngx.say("ngx.var.request_uri: ", ngx.var.request_uri)
end
+-- echo back exactly what the upstream received: the full request URI (with
+-- query string) and every request header. Lets tests assert on what was
+-- actually proxied upstream instead of scanning the error log.
Review Comment:
Grammar in comment: "Lets tests assert" is missing "the".
##########
apisix/plugins/jwt-auth/parser.lua:
##########
@@ -229,21 +229,40 @@ end
function _M.verify_claims(self, claims, conf)
- if not claims then
- claims = default_claims
+ -- When `claims_to_verify` is not configured (nil or an explicitly empty
+ -- array), fall back to the default claims (exp/nbf) and validate them only
+ -- if they are present in the payload. This closes the expired-token hole
+ -- while staying lenient for tokens that legitimately omit these claims.
+ -- An empty array must NOT skip validation, otherwise it reopens the
bypass.
+ if not claims or #claims == 0 then
+ for _, claim_name in ipairs(default_claims) do
+ local claim = self.payload[claim_name]
+ if claim ~= nil then
+ local checker = claims_checker[claim_name]
+ if type(claim) ~= checker.type then
+ return false, "claim " .. claim_name .. " is not a " ..
checker.type
+ end
+ local ok, err = checker.check(claim, conf)
+ if not ok then
+ return false, err
+ end
+ end
+ end
+
+ return true
end
+ -- When `claims_to_verify` is explicitly configured, the listed claims are
+ -- required: they must exist in the payload and be valid.
for _, claim_name in ipairs(claims) do
local claim = self.payload[claim_name]
- if claim then
- local checker = claims_checker[claim_name]
- if type(claim) ~= checker.type then
- return false, "claim " .. claim_name .. " is not a " ..
checker.type
- end
- local ok, err = checker.check(claim, conf)
- if not ok then
- return false, err
- end
+ local checker = claims_checker[claim_name]
+ if type(claim) ~= checker.type then
+ return false, "claim " .. claim_name .. " is not a " ..
checker.type
+ end
Review Comment:
When `claims_to_verify` is explicitly configured and a claim is missing from
the JWT payload, `claim` becomes nil and the error returned is "claim <name> is
not a number". This is misleading (the claim is missing, not the wrong type),
and in multi-auth mode this detailed message can propagate to callers.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
