gnodet-bot commented on code in PR #26435:
URL: https://github.com/apache/camel/pull/26435#discussion_r4013756852
##########
components/camel-spiffe/src/main/java/org/apache/camel/component/spiffe/SpiffeProducer.java:
##########
@@ -78,13 +79,38 @@ private void validateJwtSvid(WorkloadApiClient client,
Exchange exchange) throws
// the audience is the check here, not a parameter: it is what binds
the token to THIS workload, so it
// comes from the configuration only. Honouring CamelSpiffeAudience
would let a caller validate a token
// minted for someone else against an audience of their choosing.
- String[] audiences = resolveConfiguredAudiences();
- JwtSvid svid = client.validateJwtSvid(token, audiences[0]);
+ JwtSvid svid = validateAgainstAnyAudience(client, token,
resolveConfiguredAudiences());
Message message = getMessageForResponse(exchange);
message.setBody(svid);
message.setHeader(SpiffeConstants.SPIFFE_ID,
svid.getSpiffeId().toString());
}
+ /**
+ * Validates the token against the configured audiences, accepting it if
<em>any</em> of them matches.
+ * <p/>
+ * The Workload API validates against one audience at a time, so a
configured list has to be tried in turn. Taking
+ * only the first would silently enforce a narrower rule than the
configuration asks for, which is the wrong failure
+ * mode for a check that decides whether a caller is authenticated.
+ */
+ private JwtSvid validateAgainstAnyAudience(WorkloadApiClient client,
String token, String[] audiences)
+ throws JwtSvidException {
+ JwtSvidException failure = null;
+ for (String audience : audiences) {
+ try {
+ return client.validateJwtSvid(token, audience);
+ } catch (JwtSvidException e) {
+ // could be this audience, or the token itself; only once
every audience has failed do we know
+ failure = e;
+ }
+ }
+ // resolveAudiences never returns an empty array, so the loop ran and
failure is set; be explicit rather
+ // than leaving a reader (or a static analyser) to prove it
Review Comment:
🔍 **Nit — stale method name in comment:** The comment says
`resolveAudiences` but the method that supplies the `audiences` array at the
call site is `resolveConfiguredAudiences()`. Both delegate to
`splitAudiences()` so the invariant holds, but the wrong name is confusing —
especially since `resolveAudiences(exchange)` is the other method (the one that
*does* honour the header). A reader doing a quick audit of the validation path
could be misled into thinking the header is in scope here.
```suggestion
// resolveConfiguredAudiences never returns an empty array, so the
loop ran and failure is set; be explicit
// rather than leaving a reader (or a static analyser) to prove it
```
--
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]
