smolnar82 commented on code in PR #1405:
URL: https://github.com/apache/knox/pull/1405#discussion_r4034628297
##########
gateway-provider-security-jwt/src/test/java/org/apache/knox/gateway/provider/federation/jwt/filter/TokenExchangeHandlerTest.java:
##########
@@ -571,6 +560,74 @@ public void
testNoResourceOrAudienceLeavesRequestAttributeUnset() throws Excepti
assertFalse(requestedAudiencesAttr.hasCaptured());
}
+ @Test
+ public void testSameSubjectRequestedAudienceIgnoredWhenFlagDisabled() throws
Exception {
+ // Fail-safe default: for a same-subject exchange the requested audience
is dropped entirely
+ // (not conveyed downstream), so a passthrough audience validator cannot
mint an
+ // arbitrarily-audienced token without authorization. The exchange still
succeeds.
+ filter.valid.put("subtok", jwtWithAudiences("alice", "KNOXSSO",
"service-a"));
+ handler.handle(exchangeRequest("subtok", null, new String[]
{"service-a"}), response, chain);
+
+ assertTrue(filter.continued);
+ assertFalse(requestedAudiencesAttr.hasCaptured());
+ }
+
+ @Test
+ public void
testSameSubjectRequestedAudienceAuthorizedWhenSubsetOfSubjectTokenAudience()
throws Exception {
+ // Honoring on: a requested audience the subject token already carries is
authorized and
+ // conveyed, even when the subject token carries additional audiences.
+ filter.delegationSameSubjectRequestedAudienceEnabled = true;
+ filter.valid.put("subtok", jwtWithAudiences("alice", "KNOXSSO",
"service-a", "service-b", "service-c"));
+ handler.handle(exchangeRequest("subtok", null, new String[]
{"service-a"}), response, chain);
+
+ assertTrue(filter.continued);
+ assertEquals(Arrays.asList("service-a"),
requestedAudiencesAttr.getValue());
+ }
+
+ @Test
+ public void
testSameSubjectRequestedAudienceRejectedWhenNotInSubjectTokenAudience() throws
Exception {
+ // Honoring on but the requested audience is not among the subject token's
own aud claim:
+ // rejected as invalid_target and never conveyed.
+ filter.delegationSameSubjectRequestedAudienceEnabled = true;
+ filter.valid.put("subtok", jwtWithAudiences("alice", "KNOXSSO",
"service-a"));
+ handler.handle(exchangeRequest("subtok", null, new String[]
{"service-b"}), response, chain);
+
+ assertFalse(filter.continued);
+ assertEquals(HttpServletResponse.SC_BAD_REQUEST, filter.errorStatus);
+ assertEquals("invalid_target", filter.error);
+ assertFalse(requestedAudiencesAttr.hasCaptured());
+ }
+
+ @Test
+ public void
testSameSubjectRequestedAudienceRejectedWhenSubjectTokenHasNoAudience() throws
Exception {
+ // Honoring on and a requested audience present, but the subject token
carries no aud claim:
+ // there is nothing to authorize against, so the request is rejected.
+ filter.delegationSameSubjectRequestedAudienceEnabled = true;
+ filter.valid.put("subtok", jwt("alice", "KNOXSSO"));
+ handler.handle(exchangeRequest("subtok", null, new String[]
{"service-a"}), response, chain);
+
+ assertFalse(filter.continued);
+ assertEquals(HttpServletResponse.SC_BAD_REQUEST, filter.errorStatus);
+ assertEquals("invalid_target", filter.error);
+ assertFalse(requestedAudiencesAttr.hasCaptured());
+ }
Review Comment:
Done.
##########
gateway-provider-security-jwt/src/main/java/org/apache/knox/gateway/provider/federation/jwt/filter/TokenExchangeHandler.java:
##########
@@ -316,7 +316,29 @@ ActionOutcome.SUCCESS, auditMessage(policyDecision,
actorIdentity, subjectToken,
// present so that KNOXTOKEN falls back to its resource query parameter
otherwise; when set,
// the body value takes precedence over the query parameter.
if (!requestedAudiences.isEmpty()) {
-
request.setAttribute(CommonTokenConstants.REQUESTED_AUDIENCES_REQUEST_ATTR,
requestedAudiences);
+ final boolean sameSubjectExchange = !hasActorToken &&
!requestedSubjectDiffersFromSubject;
Review Comment:
Fixed.
--
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]
